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

fix(eslint-plugin): move ban-context to react config from base #23240

Merged
merged 6 commits into from
May 26, 2022

Conversation

Hotell
Copy link
Contributor

@Hotell Hotell commented May 25, 2022

Current Behavior

ban-context-export is part of base, thus is applied on node.js apps which is out of scope

New Behavior

  • ban-context is applied only on web related packages in v9
  • some additional refactoring to encapsulate type information acquisition within custom rules etc

Related Issue(s)

Follows #23141

@Hotell Hotell changed the title Hotell/lint/context rule tweaks fix(eslint-plugin): move ban-context to react config from base May 25, 2022
@codesandbox-ci
Copy link

codesandbox-ci bot commented May 25, 2022

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 59ecd76:

Sandbox Source
@fluentui/react 8 starter Configuration
@fluentui/react-components 9 starter Configuration

@fabricteam
Copy link
Collaborator

fabricteam commented May 25, 2022

Perf Analysis (@fluentui/react-components)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 913 907 5000
Button mount 576 600 5000
FluentProvider mount 1919 2241 5000
FluentProviderWithTheme mount 268 277 10
FluentProviderWithTheme virtual-rerender 247 226 10
FluentProviderWithTheme virtual-rerender-with-unmount 305 290 10
MakeStyles mount 1658 1680 50000

@fabricteam
Copy link
Collaborator

fabricteam commented May 25, 2022

📊 Bundle size report

Unchanged fixtures
Package & Exports Size (minified/GZIP)
priority-overflow
createOverflowManager
2.936 kB
1.212 kB
react-accordion
Accordion (including children components)
74.251 kB
22.683 kB
react-avatar
Avatar
45.571 kB
13.318 kB
react-badge
Badge
20.985 kB
6.62 kB
react-badge
CounterBadge
21.898 kB
6.925 kB
react-badge
PresenceBadge
22.21 kB
6.672 kB
react-button
Button
32.927 kB
9.036 kB
react-button
CompoundButton
39.854 kB
10.26 kB
react-button
MenuButton
35.11 kB
9.783 kB
react-button
SplitButton
42.369 kB
11.085 kB
react-button
ToggleButton
47.52 kB
10.603 kB
react-card
Card - All
60.746 kB
17.41 kB
react-card
Card
56.031 kB
16.147 kB
react-card
CardFooter
7.793 kB
3.327 kB
react-card
CardHeader
9.363 kB
3.841 kB
react-card
CardPreview
7.765 kB
3.35 kB
react-combobox
Combobox
61.412 kB
20.982 kB
react-components
react-components: Accordion, Button, FluentProvider, Image, Menu, Popover
181.169 kB
50.611 kB
react-components
react-components: FluentProvider & webLightTheme
34.078 kB
11.098 kB
react-divider
Divider
15.55 kB
5.595 kB
react-image
Image
10.012 kB
3.982 kB
react-input
Input
22.163 kB
7.295 kB
react-label
Label
8.57 kB
3.586 kB
react-link
Link
11.422 kB
4.648 kB
react-menu
Menu (including children components)
111.638 kB
34.058 kB
react-menu
Menu (including selectable components)
114.813 kB
34.528 kB
react-overflow
hooks only
10.792 kB
4.124 kB
react-popover
Popover
103.753 kB
31.544 kB
react-portal
Portal
6.272 kB
2.17 kB
react-positioning
usePositioning
23.828 kB
8.283 kB
react-provider
FluentProvider
14.127 kB
5.299 kB
react-radio
Radio
29.507 kB
10.108 kB
react-radio
RadioGroup
13.664 kB
5.486 kB
react-select
Select
17.138 kB
6.367 kB
react-slider
Slider
25.334 kB
8.203 kB
react-spinbutton
SpinButton
42.132 kB
11.895 kB
react-spinner
Spinner
18.132 kB
6.013 kB
react-switch
Switch
25.463 kB
8.261 kB
react-text
Text - Default
10.904 kB
4.298 kB
react-text
Text - Wrappers
14.226 kB
4.714 kB
react-textarea
Textarea
21.207 kB
7.165 kB
react-theme
Single theme token import
69 B
89 B
react-theme
Teams: all themes
31.363 kB
7.043 kB
react-theme
Teams: Light theme
19.806 kB
5.699 kB
react-tooltip
Tooltip
43.584 kB
14.992 kB
react-utilities
SSRProvider
189 B
161 B
🤖 This report was generated against 8c00b612d84dbe0f27c5c5043a2d8752cf88e8ee

@fabricteam
Copy link
Collaborator

fabricteam commented May 25, 2022

Perf Analysis (@fluentui/react-northstar)

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
AttachmentMinimalPerf.default 133 124 1.07:1
FormMinimalPerf.default 372 350 1.06:1
ChatWithPopoverPerf.default 311 297 1.05:1
DividerMinimalPerf.default 326 318 1.03:1
ImageMinimalPerf.default 338 328 1.03:1
TreeMinimalPerf.default 723 703 1.03:1
AvatarMinimalPerf.default 160 157 1.02:1
ButtonOverridesMissPerf.default 1176 1153 1.02:1
CarouselMinimalPerf.default 365 359 1.02:1
ChatDuplicateMessagesPerf.default 235 230 1.02:1
DropdownMinimalPerf.default 2537 2497 1.02:1
InputMinimalPerf.default 1010 986 1.02:1
LayoutMinimalPerf.default 323 317 1.02:1
PortalMinimalPerf.default 145 142 1.02:1
SegmentMinimalPerf.default 312 307 1.02:1
StatusMinimalPerf.default 621 606 1.02:1
IconMinimalPerf.default 532 523 1.02:1
TextAreaMinimalPerf.default 427 418 1.02:1
AnimationMinimalPerf.default 495 489 1.01:1
AttachmentSlotsPerf.default 860 854 1.01:1
ChatMinimalPerf.default 655 650 1.01:1
DatepickerMinimalPerf.default 4808 4764 1.01:1
DialogMinimalPerf.default 697 688 1.01:1
GridMinimalPerf.default 303 300 1.01:1
ItemLayoutMinimalPerf.default 1006 994 1.01:1
LabelMinimalPerf.default 349 346 1.01:1
ListCommonPerf.default 539 534 1.01:1
LoaderMinimalPerf.default 538 533 1.01:1
RosterPerf.default 873 866 1.01:1
PopupMinimalPerf.default 563 555 1.01:1
ProviderMergeThemesPerf.default 1000 994 1.01:1
SplitButtonMinimalPerf.default 3357 3314 1.01:1
TableManyItemsPerf.default 1640 1630 1.01:1
CustomToolbarPrototype.default 2209 2198 1.01:1
ToolbarMinimalPerf.default 845 834 1.01:1
AlertMinimalPerf.default 226 227 1:1
CheckboxMinimalPerf.default 1962 1962 1:1
EmbedMinimalPerf.default 2983 2991 1:1
HeaderSlotsPerf.default 681 680 1:1
ListNestedPerf.default 473 471 1:1
MenuButtonMinimalPerf.default 1369 1366 1:1
RadioGroupMinimalPerf.default 398 397 1:1
ReactionMinimalPerf.default 333 333 1:1
SliderMinimalPerf.default 1312 1317 1:1
TreeWith60ListItems.default 126 126 1:1
ButtonMinimalPerf.default 133 134 0.99:1
CardMinimalPerf.default 488 494 0.99:1
ListMinimalPerf.default 458 463 0.99:1
MenuMinimalPerf.default 748 752 0.99:1
RefMinimalPerf.default 206 208 0.99:1
TableMinimalPerf.default 359 363 0.99:1
TooltipMinimalPerf.default 968 982 0.99:1
VideoMinimalPerf.default 567 573 0.99:1
BoxMinimalPerf.default 293 298 0.98:1
DropdownManyItemsPerf.default 551 565 0.98:1
FlexMinimalPerf.default 242 247 0.98:1
HeaderMinimalPerf.default 313 321 0.98:1
ListWith60ListItems.default 472 484 0.98:1
ProviderMinimalPerf.default 312 318 0.98:1
TextMinimalPerf.default 302 309 0.98:1
AccordionMinimalPerf.default 127 131 0.97:1
ButtonSlotsPerf.default 416 433 0.96:1
SkeletonMinimalPerf.default 305 318 0.96:1

@fabricteam
Copy link
Collaborator

fabricteam commented May 25, 2022

Perf Analysis (@fluentui/react)

Scenario Render type Master Ticks PR Ticks Iterations Status
buttonNative mount 91 119 5000 Possible regression
All results

Scenario Render type Master Ticks PR Ticks Iterations Status
BaseButton mount 790 781 5000
Breadcrumb mount 2259 2260 1000
Checkbox mount 1275 1258 5000
CheckboxBase mount 1073 1083 5000
ChoiceGroup mount 3910 3921 5000
ComboBox mount 749 764 1000
CommandBar mount 8829 8883 1000
ContextualMenu mount 9198 9200 1000
DefaultButton mount 975 964 5000
DetailsRow mount 3055 3066 5000
DetailsRowFast mount 3082 3051 5000
DetailsRowNoStyles mount 2917 2894 5000
Dialog mount 1862 1888 1000
DocumentCardTitle mount 151 156 1000
Dropdown mount 2753 2787 5000
FocusTrapZone mount 1550 1517 5000
FocusZone mount 1530 1553 5000
IconButton mount 1393 1394 5000
Label mount 328 315 5000
Layer mount 2543 2567 5000
Link mount 423 432 5000
MenuButton mount 1222 1208 5000
MessageBar mount 1861 1802 5000
Nav mount 2643 2636 1000
OverflowSet mount 987 1000 5000
Panel mount 1826 1855 1000
Persona mount 858 864 1000
Pivot mount 1118 1113 1000
PrimaryButton mount 1090 1081 5000
Rating mount 6659 6612 5000
SearchBox mount 1121 1105 5000
Shimmer mount 2227 2222 5000
Slider mount 1704 1706 5000
SpinButton mount 3947 3952 5000
Spinner mount 401 393 5000
SplitButton mount 2463 2500 5000
Stack mount 480 489 5000
StackWithIntrinsicChildren mount 1860 1849 5000
StackWithTextChildren mount 4614 4634 5000
SwatchColorPicker mount 9028 9080 5000
TagPicker mount 1995 2039 5000
TeachingBubble mount 70442 70458 5000
Text mount 388 400 5000
TextField mount 1184 1165 5000
ThemeProvider mount 944 943 5000
ThemeProvider virtual-rerender 589 609 5000
ThemeProvider virtual-rerender-with-unmount 1423 1410 5000
Toggle mount 702 703 5000
buttonNative mount 91 119 5000 Possible regression

@size-auditor
Copy link

size-auditor bot commented May 25, 2022

Asset size changes

Size Auditor did not detect a change in bundle size for any component!

Baseline commit: 8c00b612d84dbe0f27c5c5043a2d8752cf88e8ee (build)

identifierName = identifier.name;
const identifierName = identifier.name;

checkContextType(declaration, identifierName);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

early exit to mitigate mutations and scope leaks

@@ -38,8 +38,6 @@ ruleTester.run('ban-context-export', rule, {
filename: 'src/index.ts',
},
{
// No way to type generics with jsdoc
// @ts-ignore
options: [{ exclude: ['**/special-path/**/*'] }],
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this now works

fileNameCheck:(fileName:string)=>boolean;
}} options
*/
function reportViolation(options) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved logic to this generic function to check violations, as the code was quite similar

/**
*
* @param {{
typeProperty: Extract<keyof import('typescript').Type, 'symbol' | 'aliasSymbol'>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this enables better API contract with 3rd party types as if our defined literals wouldn't exist, we would get type error later on when used

module.exports = createRule({
name: 'ban-context-export',
defaultOptions: [],
defaultOptions: /** @type {ReadonlyArray<Options>} */ ([{}]),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with this createRule properly instantiates context generic, thus type intellisense works as expected (even in tests)

@Hotell Hotell marked this pull request as ready for review May 25, 2022 14:27
@Hotell Hotell requested a review from a team as a code owner May 25, 2022 14:27
@Hotell Hotell requested a review from ling1726 May 25, 2022 14:27

/**
*
* @param {{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd add some additional doc to this param to explain what the two different symbols are

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for pushing this dude, thanks to this I finally understood the difference 🥇

docs added 59ecd76

@Hotell Hotell merged commit 680885f into microsoft:master May 26, 2022
@Hotell Hotell deleted the hotell/lint/context-rule-tweaks branch May 26, 2022 11:06
marwan38 pushed a commit to marwan38/fluentui that referenced this pull request Jun 13, 2022
…soft#23240)

* fix(eslint-plugin): move ban-context to react config from base

* refactor(eslint-plugin): improve type inference and encapsulate some logic in ban-context

* generate change files

* fixup! fix(eslint-plugin): move ban-context to react config from base

* fixup! refactor(eslint-plugin): improve type inference and encapsulate some logic in ban-context

* fixup! fixup! refactor(eslint-plugin): improve type inference and encapsulate some logic in ban-context
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants