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

feat(eslint-plugin): replacace deprecation/deprecation with etc/no-deprecated #22251

Conversation

Hotell
Copy link
Contributor

@Hotell Hotell commented Mar 30, 2022

prerequisite:

Current Behavior

We approach deprecated API's via lint rule that triggers lint errors for following use cases:

// @filename tools.ts

/**
 * @deprecated use world instead
 */
export const hello = 'world'

export const world = 'this one'

↓↓↓

// @filename usage.ts

import {hello} from './tools'

// 🚨 lint error - 'hello' is deprecated. [object Object] 
export const greeter = () => `${hello} folks`

↓↓↓

// @filename public-api.ts

export {
  // 🚨 lint error -  'hello' is deprecated. [object Object] 
  hello, 
  world
} from './tools'

New Behavior

// @filename tools.ts

/**
 * @deprecated use world instead
 */
export const hello = 'world'

export const world = 'this one'

↓↓↓

// @filename usage.ts

import {hello} from './tools'

// 🚨 lint error - "hello" is deprecated: use world instead
export const greeter = () => `${hello} folks`

↓↓↓

// @filename public-api.ts

export { 
  hello, 
  world
} from './tools'

comparison

deprecation/deprecation etc/no-deprecated
perf benchmark image image
report message 'hello' is deprecated. [object Object] "hello" is deprecated: use world instead
usage errors report
export token report
properties report

summary

This PR:

  • etc/no-deprecated is 5 times faster 🔥
  • replacing existing plugin/rule won't affect consumers in any way
  • issue(s) that came up on yesterdays sync:
    • lint won't stop you to export deprecated identifiers from your package. This has 2 sides. you export something as part of public API. later you add @deprecated. Now you cannot remove it from public api as that would be Breaking change - you need to use eslint-ignore pragma - not the best approach IMO.

Related Issue(s)

cartant/eslint-plugin-etc#43

@codesandbox-ci
Copy link

codesandbox-ci bot commented Mar 30, 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 967d95c:

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

@size-auditor
Copy link

size-auditor bot commented Mar 30, 2022

Asset size changes

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

Baseline commit: 905e46db07ada986fd7885531f72a71b7bbaabdc (build)

@fabricteam
Copy link
Collaborator

fabricteam commented Mar 30, 2022

Perf Analysis (@fluentui/react-components)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 1078 1205 5000
Button mount 669 647 5000
FluentProvider mount 2207 2180 5000
FluentProviderWithTheme mount 313 345 10
FluentProviderWithTheme virtual-rerender 277 297 10
FluentProviderWithTheme virtual-rerender-with-unmount 416 397 10
MakeStyles mount 1850 1925 50000

@fabricteam
Copy link
Collaborator

fabricteam commented Mar 30, 2022

📊 Bundle size report

Unchanged fixtures
Package & Exports Size (minified/GZIP)
priority-overflow
createOverflowManager
2.836 kB
1.209 kB
react-accordion
Accordion (including children components)
74.792 kB
22.516 kB
react-avatar
Avatar
45.142 kB
13.111 kB
react-badge
Badge
20.895 kB
6.567 kB
react-badge
CounterBadge
21.848 kB
6.883 kB
react-badge
PresenceBadge
21.951 kB
6.565 kB
react-button
Button
28.013 kB
8.059 kB
react-button
CompoundButton
33.508 kB
9.092 kB
react-button
MenuButton
29.796 kB
8.665 kB
react-button
SplitButton
36.268 kB
9.863 kB
react-button
ToggleButton
37.395 kB
8.68 kB
react-card
Card - All
53.619 kB
15.372 kB
react-card
Card
48.904 kB
14.089 kB
react-card
CardFooter
7.686 kB
3.264 kB
react-card
CardHeader
9.251 kB
3.78 kB
react-card
CardPreview
7.658 kB
3.291 kB
react-combobox
Combobox
6.817 kB
2.901 kB
react-components
react-components: Accordion, Button, FluentProvider, Image, Menu, Popover
176.089 kB
49.113 kB
react-components
react-components: FluentProvider & webLightTheme
32.526 kB
10.645 kB
react-divider
Divider
15.385 kB
5.539 kB
react-image
Image
10.109 kB
3.958 kB
react-input
Input
21.661 kB
7.18 kB
react-label
Label
8.371 kB
3.504 kB
react-link
Link
11.106 kB
4.507 kB
react-menu
Menu (including children components)
105.852 kB
32.433 kB
react-menu
Menu (including selectable components)
109.031 kB
32.897 kB
react-popover
Popover
96.787 kB
29.559 kB
react-portal
Portal
6.272 kB
2.17 kB
react-positioning
usePopper
23.21 kB
8.084 kB
react-priority-overflow
hooks only
10.606 kB
4.087 kB
react-provider
FluentProvider
14.009 kB
5.25 kB
react-select
Select
16.562 kB
6.264 kB
react-slider
Slider
25.549 kB
8.25 kB
react-spinner
Spinner
6.815 kB
2.9 kB
react-switch
Switch
24.279 kB
8.001 kB
react-text
Text - Default
10.797 kB
4.233 kB
react-text
Text - Wrappers
14.113 kB
4.576 kB
react-theme
Single theme token import
69 B
89 B
react-theme
Teams: all themes
29.426 kB
6.551 kB
react-theme
Teams: Light theme
18.42 kB
5.27 kB
react-tooltip
Tooltip
42.837 kB
14.727 kB
react-utilities
SSRProvider
189 B
161 B
🤖 This report was generated against 905e46db07ada986fd7885531f72a71b7bbaabdc

@fabricteam
Copy link
Collaborator

fabricteam commented Mar 30, 2022

Perf Analysis (@fluentui/react)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
BaseButton mount 952 963 5000
Breadcrumb mount 2835 2776 1000
Checkbox mount 1502 1526 5000
CheckboxBase mount 1314 1280 5000
ChoiceGroup mount 4836 4712 5000
ComboBox mount 1058 1028 1000
CommandBar mount 10741 10855 1000
ContextualMenu mount 12247 12184 1000
DefaultButton mount 1160 1168 5000
DetailsRow mount 3849 3934 5000
DetailsRowFast mount 3986 3880 5000
DetailsRowNoStyles mount 3742 3689 5000
Dialog mount 2242 2276 1000
DocumentCardTitle mount 164 169 1000
Dropdown mount 3346 3389 5000
FocusTrapZone mount 1866 1830 5000
FocusZone mount 1889 1897 5000
IconButton mount 1793 1791 5000
Label mount 388 354 5000
Layer mount 3089 3016 5000
Link mount 474 484 5000
MenuButton mount 1476 1497 5000
MessageBar mount 2203 2223 5000
Nav mount 3358 3372 1000
OverflowSet mount 1120 1127 5000
Panel mount 2212 2182 1000
Persona mount 1011 1003 1000
Pivot mount 1587 1499 1000
PrimaryButton mount 1303 1354 5000
Rating mount 7989 7902 5000
SearchBox mount 1303 1366 5000
Shimmer mount 2525 2523 5000
Slider mount 1962 1938 5000
SpinButton mount 5429 5109 5000
Spinner mount 434 448 5000
SplitButton mount 3263 3240 5000
Stack mount 536 560 5000
StackWithIntrinsicChildren mount 2341 2313 5000
StackWithTextChildren mount 5256 5293 5000
SwatchColorPicker mount 11919 11933 5000
TagPicker mount 2718 2737 5000
TeachingBubble mount 100657 100378 5000
Text mount 413 440 5000
TextField mount 1449 1488 5000
ThemeProvider mount 1196 1202 5000
ThemeProvider virtual-rerender 656 664 5000
ThemeProvider virtual-rerender-with-unmount 1871 1916 5000
Toggle mount 837 811 5000
buttonNative mount 127 127 5000

@fabricteam
Copy link
Collaborator

fabricteam commented Mar 30, 2022

Perf Analysis (@fluentui/react-northstar)

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
ReactionMinimalPerf.default 325 277 1.17:1
LabelMinimalPerf.default 330 294 1.12:1
TableManyItemsPerf.default 1615 1470 1.1:1
ButtonMinimalPerf.default 161 148 1.09:1
TreeWith60ListItems.default 166 154 1.08:1
AttachmentSlotsPerf.default 916 857 1.07:1
RosterPerf.default 956 891 1.07:1
PopupMinimalPerf.default 538 501 1.07:1
DividerMinimalPerf.default 315 296 1.06:1
LayoutMinimalPerf.default 319 302 1.06:1
MenuButtonMinimalPerf.default 1416 1333 1.06:1
CardMinimalPerf.default 487 466 1.05:1
FormMinimalPerf.default 348 331 1.05:1
HeaderMinimalPerf.default 321 306 1.05:1
TextMinimalPerf.default 285 271 1.05:1
ChatWithPopoverPerf.default 345 332 1.04:1
FlexMinimalPerf.default 258 249 1.04:1
ProviderMinimalPerf.default 984 946 1.04:1
TextAreaMinimalPerf.default 397 380 1.04:1
CarouselMinimalPerf.default 405 395 1.03:1
ChatDuplicateMessagesPerf.default 272 265 1.03:1
HeaderSlotsPerf.default 647 626 1.03:1
LoaderMinimalPerf.default 585 570 1.03:1
PortalMinimalPerf.default 154 149 1.03:1
SplitButtonMinimalPerf.default 3620 3529 1.03:1
AnimationMinimalPerf.default 471 461 1.02:1
ImageMinimalPerf.default 319 314 1.02:1
InputMinimalPerf.default 1115 1093 1.02:1
SkeletonMinimalPerf.default 298 291 1.02:1
IconMinimalPerf.default 498 490 1.02:1
AccordionMinimalPerf.default 135 134 1.01:1
BoxMinimalPerf.default 295 293 1.01:1
ChatMinimalPerf.default 642 637 1.01:1
CheckboxMinimalPerf.default 2263 2248 1.01:1
ListCommonPerf.default 535 530 1.01:1
ListNestedPerf.default 471 467 1.01:1
ListWith60ListItems.default 536 530 1.01:1
RefMinimalPerf.default 208 206 1.01:1
CustomToolbarPrototype.default 3524 3488 1.01:1
ButtonSlotsPerf.default 465 467 1:1
EmbedMinimalPerf.default 3483 3499 1:1
GridMinimalPerf.default 286 285 1:1
ItemLayoutMinimalPerf.default 989 985 1:1
ListMinimalPerf.default 426 428 1:1
RadioGroupMinimalPerf.default 374 374 1:1
ToolbarMinimalPerf.default 817 813 1:1
TooltipMinimalPerf.default 864 867 1:1
TreeMinimalPerf.default 690 690 1:1
VideoMinimalPerf.default 525 523 1:1
DatepickerMinimalPerf.default 4850 4888 0.99:1
DropdownMinimalPerf.default 2526 2560 0.99:1
TableMinimalPerf.default 341 344 0.99:1
AlertMinimalPerf.default 235 240 0.98:1
DropdownManyItemsPerf.default 561 575 0.98:1
ProviderMergeThemesPerf.default 1434 1470 0.98:1
SliderMinimalPerf.default 1388 1423 0.98:1
DialogMinimalPerf.default 619 640 0.97:1
ButtonOverridesMissPerf.default 1394 1446 0.96:1
SegmentMinimalPerf.default 278 294 0.95:1
AvatarMinimalPerf.default 155 169 0.92:1
MenuMinimalPerf.default 648 724 0.9:1
StatusMinimalPerf.default 509 569 0.89:1
AttachmentMinimalPerf.default 121 139 0.87:1

@Hotell Hotell changed the title Hotell/eslint plugin/faster deprecate rule feat(eslint-plugin): replacace deprecation/deprecation with etc/no-deprecated Mar 30, 2022
@ecraig12345
Copy link
Member

I think it’s not that big of a deal to get rid of warnings about deprecated exports. What I’m really curious about is how the new deprecation rule is so much faster. I made some major contributions to eslint-plugin-deprecation (porting over additional capabilities from tslint, adding better JSX attribute support, adding tests) and I knew the perf was terrible, but I didn’t know how to optimize it since I still haven’t been able to build a good mental model of how the typescript compiler API works (so I don’t have a good sense of what’s most expensive and how to avoid it).

@msft-fluent-ui-bot
Copy link
Collaborator

Because this pull request has not had activity for over 150 days, we're automatically closing it for house-keeping purposes.

The pull request will still be available for reference. If it's still relevant to merge at some point, you can reopen or make a new version based on the latest code.

@msft-fluent-ui-bot msft-fluent-ui-bot added the Resolution: Soft Close Soft closing inactive issues over a certain period label Aug 29, 2022
@Hotell
Copy link
Contributor Author

Hotell commented Aug 29, 2022

I think it’s not that big of a deal to get rid of warnings about deprecated exports. What I’m really curious about is how the new deprecation rule is so much faster. I made some major contributions to eslint-plugin-deprecation (porting over additional capabilities from tslint, adding better JSX attribute support, adding tests) and I knew the perf was terrible, but I didn’t know how to optimize it since I still haven’t been able to build a good mental model of how the typescript compiler API works (so I don’t have a good sense of what’s most expensive and how to avoid it).

feel free to check the whole implementation https://github.com/cartant/eslint-plugin-etc/blob/main/source/rules/no-deprecated.ts

looks like the perf tricks lies within this one cartant/eslint-plugin-etc#10 (comment)

@Hotell Hotell reopened this Aug 29, 2022
@github-actions github-actions bot added this to the July Project Cycle Q3 2022 milestone Aug 29, 2022
@msft-fluent-ui-bot
Copy link
Collaborator

Because this pull request has not had activity for over 150 days, we're automatically closing it for house-keeping purposes.

The pull request will still be available for reference. If it's still relevant to merge at some point, you can reopen or make a new version based on the latest code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Soft Close Soft closing inactive issues over a certain period
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants