-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
fix(eslint-plugin): make import/no-extraneous-dependencies work for react config #21924
fix(eslint-plugin): make import/no-extraneous-dependencies work for react config #21924
Conversation
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 0bcc3bc:
|
Asset size changesSize Auditor did not detect a change in bundle size for any component! Baseline commit: e6fa9864d66e5cdf4e965b6069f28c4455dd35dc (build) |
📊 Bundle size reportUnchanged fixtures
|
Perf Analysis (
|
Scenario | Current PR Ticks | Baseline Ticks | Ratio |
---|---|---|---|
AvatarMinimalPerf.default | 178 | 167 | 1.07:1 |
RadioGroupMinimalPerf.default | 394 | 374 | 1.05:1 |
SegmentMinimalPerf.default | 300 | 287 | 1.05:1 |
ChatMinimalPerf.default | 638 | 616 | 1.04:1 |
ListMinimalPerf.default | 443 | 425 | 1.04:1 |
TableMinimalPerf.default | 352 | 339 | 1.04:1 |
FlexMinimalPerf.default | 251 | 243 | 1.03:1 |
GridMinimalPerf.default | 296 | 286 | 1.03:1 |
HeaderMinimalPerf.default | 312 | 303 | 1.03:1 |
LayoutMinimalPerf.default | 314 | 306 | 1.03:1 |
TreeWith60ListItems.default | 155 | 150 | 1.03:1 |
FormMinimalPerf.default | 352 | 344 | 1.02:1 |
HeaderSlotsPerf.default | 646 | 631 | 1.02:1 |
LabelMinimalPerf.default | 327 | 321 | 1.02:1 |
RosterPerf.default | 984 | 960 | 1.02:1 |
ReactionMinimalPerf.default | 319 | 313 | 1.02:1 |
RefMinimalPerf.default | 207 | 202 | 1.02:1 |
StatusMinimalPerf.default | 574 | 562 | 1.02:1 |
TableManyItemsPerf.default | 1620 | 1596 | 1.02:1 |
ToolbarMinimalPerf.default | 816 | 797 | 1.02:1 |
TreeMinimalPerf.default | 689 | 675 | 1.02:1 |
AnimationMinimalPerf.default | 468 | 463 | 1.01:1 |
ButtonOverridesMissPerf.default | 1440 | 1426 | 1.01:1 |
ButtonSlotsPerf.default | 470 | 467 | 1.01:1 |
CheckboxMinimalPerf.default | 2259 | 2242 | 1.01:1 |
DividerMinimalPerf.default | 304 | 301 | 1.01:1 |
InputMinimalPerf.default | 1102 | 1091 | 1.01:1 |
LoaderMinimalPerf.default | 581 | 577 | 1.01:1 |
SkeletonMinimalPerf.default | 300 | 296 | 1.01:1 |
SplitButtonMinimalPerf.default | 3693 | 3654 | 1.01:1 |
TextMinimalPerf.default | 295 | 293 | 1.01:1 |
AlertMinimalPerf.default | 234 | 233 | 1:1 |
CarouselMinimalPerf.default | 409 | 407 | 1:1 |
DropdownManyItemsPerf.default | 566 | 565 | 1:1 |
DropdownMinimalPerf.default | 2585 | 2575 | 1:1 |
EmbedMinimalPerf.default | 3479 | 3492 | 1:1 |
ItemLayoutMinimalPerf.default | 993 | 993 | 1:1 |
ListNestedPerf.default | 467 | 466 | 1:1 |
MenuMinimalPerf.default | 713 | 716 | 1:1 |
MenuButtonMinimalPerf.default | 1409 | 1402 | 1:1 |
PortalMinimalPerf.default | 156 | 156 | 1:1 |
ProviderMergeThemesPerf.default | 1458 | 1460 | 1:1 |
ProviderMinimalPerf.default | 972 | 969 | 1:1 |
SliderMinimalPerf.default | 1427 | 1420 | 1:1 |
TextAreaMinimalPerf.default | 403 | 405 | 1:1 |
TooltipMinimalPerf.default | 865 | 866 | 1:1 |
VideoMinimalPerf.default | 550 | 548 | 1:1 |
AttachmentMinimalPerf.default | 137 | 138 | 0.99:1 |
DatepickerMinimalPerf.default | 4616 | 4655 | 0.99:1 |
DialogMinimalPerf.default | 639 | 644 | 0.99:1 |
PopupMinimalPerf.default | 528 | 531 | 0.99:1 |
CustomToolbarPrototype.default | 3428 | 3458 | 0.99:1 |
BoxMinimalPerf.default | 287 | 294 | 0.98:1 |
CardMinimalPerf.default | 454 | 463 | 0.98:1 |
ChatDuplicateMessagesPerf.default | 252 | 258 | 0.98:1 |
ListWith60ListItems.default | 549 | 561 | 0.98:1 |
IconMinimalPerf.default | 507 | 518 | 0.98:1 |
AttachmentSlotsPerf.default | 903 | 935 | 0.97:1 |
ChatWithPopoverPerf.default | 332 | 343 | 0.97:1 |
ImageMinimalPerf.default | 303 | 311 | 0.97:1 |
AccordionMinimalPerf.default | 133 | 138 | 0.96:1 |
ButtonMinimalPerf.default | 139 | 146 | 0.95:1 |
ListCommonPerf.default | 519 | 554 | 0.94:1 |
Perf Analysis (
|
Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
---|---|---|---|---|---|
BaseButton | mount | 940 | 952 | 5000 | |
Breadcrumb | mount | 2717 | 2687 | 1000 | |
Checkbox | mount | 1553 | 1545 | 5000 | |
CheckboxBase | mount | 1283 | 1267 | 5000 | |
ChoiceGroup | mount | 4846 | 4763 | 5000 | |
ComboBox | mount | 1008 | 1000 | 1000 | |
CommandBar | mount | 10450 | 10537 | 1000 | |
ContextualMenu | mount | 11719 | 11760 | 1000 | |
DefaultButton | mount | 1141 | 1158 | 5000 | |
DetailsRow | mount | 3904 | 3926 | 5000 | |
DetailsRowFast | mount | 3894 | 3879 | 5000 | |
DetailsRowNoStyles | mount | 3724 | 3722 | 5000 | |
Dialog | mount | 2231 | 2299 | 1000 | |
DocumentCardTitle | mount | 165 | 169 | 1000 | |
Dropdown | mount | 3293 | 3332 | 5000 | |
FocusTrapZone | mount | 1813 | 1833 | 5000 | |
FocusZone | mount | 1867 | 1777 | 5000 | |
IconButton | mount | 1777 | 1790 | 5000 | |
Label | mount | 357 | 354 | 5000 | |
Layer | mount | 2962 | 3064 | 5000 | |
Link | mount | 479 | 473 | 5000 | |
MenuButton | mount | 1504 | 1487 | 5000 | |
MessageBar | mount | 2176 | 2126 | 5000 | |
Nav | mount | 3400 | 3390 | 1000 | |
OverflowSet | mount | 1107 | 1105 | 5000 | |
Panel | mount | 2196 | 2170 | 1000 | |
Persona | mount | 855 | 857 | 1000 | |
Pivot | mount | 1506 | 1485 | 1000 | |
PrimaryButton | mount | 1345 | 1302 | 5000 | |
Rating | mount | 7883 | 7933 | 5000 | |
SearchBox | mount | 1338 | 1345 | 5000 | |
Shimmer | mount | 2509 | 2501 | 5000 | |
Slider | mount | 1995 | 1935 | 5000 | |
SpinButton | mount | 5121 | 5105 | 5000 | |
Spinner | mount | 428 | 449 | 5000 | |
SplitButton | mount | 3282 | 3262 | 5000 | |
Stack | mount | 548 | 531 | 5000 | |
StackWithIntrinsicChildren | mount | 2357 | 2358 | 5000 | |
StackWithTextChildren | mount | 5321 | 5368 | 5000 | |
SwatchColorPicker | mount | 11910 | 11911 | 5000 | |
TagPicker | mount | 2786 | 2752 | 5000 | |
TeachingBubble | mount | 93033 | 93435 | 5000 | |
Text | mount | 432 | 430 | 5000 | |
TextField | mount | 1423 | 1436 | 5000 | |
ThemeProvider | mount | 1188 | 1187 | 5000 | |
ThemeProvider | virtual-rerender | 654 | 639 | 5000 | |
ThemeProvider | virtual-rerender-with-unmount | 1998 | 1939 | 5000 | |
Toggle | mount | 807 | 804 | 5000 | |
buttonNative | mount | 127 | 119 | 5000 |
838ea34
to
3868fba
Compare
'@typescript-eslint/parser': ['.ts', '.tsx'], | ||
}, | ||
'import/resolver': { | ||
typescript: { |
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 configuration was breaking import/no-extraneous-dependencies
to work. now that typescript
plugin is not being used at all and recommended configuration provided by import
plugin is being used instead.
/** | ||
* | ||
* import plugin rules | ||
* @see https://github.com/import-js/eslint-plugin-import |
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.
collocated all import plugins rules to one place so its easier to navigate/read
276c9f1
to
2f63a34
Compare
@@ -12,7 +12,6 @@ | |||
"license": "MIT", | |||
"devDependencies": { | |||
"@fluentui/eslint-plugin": "*", | |||
"@fluentui/scripts": "^1.0.0", | |||
"@microsoft/eslint-plugin-sdl": "0.1.9" |
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.
applied single version policy for devDeps
}, | ||
"dependencies": { | ||
"@fluentui/react-conformance": "^0.10.1", |
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.
todo: create issue to get rid of v8 deps in v9 tree
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.
@@ -409,6 +408,14 @@ | |||
"ci-info", | |||
"node-fetch" | |||
] | |||
}, | |||
{ |
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.
version group needed, because we cannot specify *
as proper dependency version
@@ -277,24 +250,40 @@ const config = { | |||
'react/no-unused-prop-types': 'off', | |||
'react/prefer-es6-class': 'off', | |||
|
|||
'jsdoc/check-tag-names': [ |
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.
no changes here - this occurs as diff because moving of lines....
@@ -1,5 +1,5 @@ | |||
{ | |||
"copyTo": { | |||
"unstable": ["./src/unstable/package.json"] | |||
"unstable/package.json": "./src/unstable/package.json__tmpl__" |
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.
leveraging new API : copy and rename to destination key
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.
package.json is using tmpl
suffix used by nx, but there are no variables being interpolated within the file. the main purpose of this is so import/no-extreanous won't check this file ( which would trigger lint errors that are invalid )
@@ -1,4 +0,0 @@ | |||
{ |
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 file is not needed, unstable
is already in check path of tsconfig.lib.json
}, | ||
"dependencies": { | ||
"@fluentui/react-conformance": "^0.10.1", |
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.
@@ -166,7 +166,6 @@ const useContentStyles = makeStyles({ | |||
/** | |||
* Apply styling to the Tab slots based on the state | |||
*/ | |||
// eslint-disable-next-line @typescript-eslint/naming-convention |
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.
🧹 removed unused lint ignores
|
||
export function expandSourcePath(pattern) { | ||
export function expandSourcePath(pattern: string): string { |
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.
improved types
); | ||
const destinationPath = path.resolve(process.cwd(), destination); | ||
|
||
if (Array.isArray(sources)) { |
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.
now there is new api:
- the old one works as before ( value is a array of string paths/globs)
- the new one is
destination
is the path including new file name + value is only string path (copy + rename)
0cb8044
to
2fb4bd5
Compare
had great brainstorming with @layershifter today and we wanna add back |
088ed06
to
570c756
Compare
…eous-dependencies work for react config
change/@fluentui-react-accordion-a88290b3-b03a-40b9-a8cc-d24ab80c5071.json
Outdated
Show resolved
Hide resolved
change/@fluentui-react-avatar-c63280cc-55c6-4360-aa31-5ced2bd439ae.json
Outdated
Show resolved
Hide resolved
change/@fluentui-react-conformance-griffel-7dbc04a7-7597-4437-a96b-cbde034b0fe0.json
Outdated
Show resolved
Hide resolved
change/@fluentui-react-tabs-7eea6ae2-201d-49d4-8749-15add0838235.json
Outdated
Show resolved
Hide resolved
change/@fluentui-react-components-6b800570-deea-41a1-a565-0987a0589347.json
Outdated
Show resolved
Hide resolved
change/@fluentui-react-provider-c610d38f-97b5-4686-ab4d-ec59b20d764a.json
Outdated
Show resolved
Hide resolved
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.
Great job, thanks for fixing this ❤️
Look at your example screenshots, should we have a Prettier/eslint rule that merges the two |
Co-authored-by: Oleksandr Fediashov <[email protected]>
Perf Analysis (
|
Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
---|---|---|---|---|---|
Avatar | mount | 1286 | 1283 | 5000 | |
Button | mount | 763 | 760 | 5000 | |
FluentProvider | mount | 2268 | 2141 | 5000 | |
FluentProviderWithTheme | mount | 394 | 400 | 10 | |
FluentProviderWithTheme | virtual-rerender | 349 | 338 | 10 | |
FluentProviderWithTheme | virtual-rerender-with-unmount | 428 | 453 | 10 | |
MakeStyles | mount | 1999 | 1916 | 50000 |
Current Behavior
New Behavior
additional feature implemented
copy
task, adds new api tocopyTo
( copy and rename )Related Issue(s)
Fixes #21789