Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat(eslint-plugin-react-components): add prefer-fluentui-v9 rule #33449
base: master
Are you sure you want to change the base?
feat(eslint-plugin-react-components): add prefer-fluentui-v9 rule #33449
Changes from 3 commits
76b38be
4943725
e948ffb
f22fbc1
944299d
ea763d8
2d8c8c8
0386842
5b6fd48
92883c4
75f0a93
6fe0ab5
9d23b9d
86d62e6
0679d93
4a9659a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wondering if this is good idea at this stage - any kind of modifications of config presets will be a breaking change for users. personally I'm fine having it as we properly follow semver.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The preset usage is entirely optional. Consumers can always switch to individual rules if they prefer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah what I meant is scenario once someone adds recommended preset to application - and we introduce BC they will be affected ( with picked rules manually not so much )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will not work for lazy imports ( not sure how often that appears in user land with fluent but we should add it as a follow-up)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on the example you gave, it's tough to do a static analysis to figure out which v8 components are used and suggest v9 alternatives. It might be doable, but I'm not sure if it's worth the effort. We could add a warning with a generic message, but I'm not sure how useful that would be.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
implementation/effort wise it's +- same logic with additional parse of destructured symbol but for different AST query selector. There is no additional complexity behind it.
users might use lazy loading modules in any kind of Suspense context or for lazy loaded application routes for perf reasons.
example: calendar is non trivial component having huge bundle size impact, in application context you don't wanna have it part of default bundle.
i think it is worth it to cover this scenario. Regarding internal users of v8 and lazy loading we need more data indeed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wondering if we could generate this list by automation and update when we trigger stable release for v9 controll
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can consider adding components to the map during the transition from preview to stable. However, we still need to define the relationship between the v8 and v9 components.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah some json metadata file per project sounds like good pattern ( which can be than leveraged for any kind of automations like this - I started implementing something like that for triage automation, will share more info later )
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: it would be probably better to use 1 Schema ( more verbose ) so every value has component and package.
that would remove need of
getMigrationDetails
completely