Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature]: Add conformance test to ensure context exports are added to the package #31329

Open
1 task done
sopranopillow opened this issue May 9, 2024 · 2 comments
Open
1 task done

Comments

@sopranopillow
Copy link
Contributor

Library

React Components / v9 (@fluentui/react-components)

Describe the feature that you would like added

After talking with some partners it is apparent that when the partner is recomposing components and the component's API later adds a context (adding usecontextvalues) to the component definition, it blocks recomposition (more important bumping versions of fluent) by not exporting these values. We are humans so this mistake can happen, but this proves to be a bad DX for partners.

Have you discussed this feature with our team

No

Additional context

Related: #31328

Validations

  • Check that there isn't already an issue that request the same feature to avoid creating a duplicate.

Priority

High

@Hotell
Copy link
Contributor

Hotell commented May 10, 2024

This is rather tricky IMO, it should be generalized to "package exports public api surface" rather than "context only - which is not present in every pacakge", but in order to determine that we need to ask a question, what is public API surface and how can we determine that ?

approach that I can think of right awau is to have a 2 parts rule:

  • explicitly annotate every token/function via @public JSDoc annotation which is supposed to be exported from package public API, and check index.ts if it contains that token
  • general rule of V9 framework public API exports (render,style,state hooks) and based on that check index.ts if these symbols are exported. while we can leverage this one based on naming, that seems to be brittle long term so again probably some kind of metadata annotation via JSDoc should be implemented

Sidenote: this should not be part of conformance test as the overall solution should not be expanded rather ported to custom workspace eslint rules that provides significantly better DX and is much more performant, obviously some patterns can be challenging/impossible to implemented on Eslint side ( probably my 1st rule which would need to parse all sources, gather symbols containing @public JSDOC and compare with export tokens within index.ts ) .

WDYT ?

@sopranopillow
Copy link
Contributor Author

Yeah I think the 1st options sounds the best path. iirc we were going to do this before, but can't remember why we ended up not using those tags. While it does seem like a drag to add this, it would greatly improve DX for partners that use our components by recomposing. Also like you pointed out, the 2nd option may bring issues along the road so I think we should stay away from that kind of path.

Thanks for the side note :) I always immediately think conformance tests, but maybe I should say conformance rules(?) Just want o make sure all components follow this set of rules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants