-
Notifications
You must be signed in to change notification settings - Fork 2.9k
feat(scripts): improve API violation reporting #25356
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(scripts): improve API violation reporting #25356
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 d8e1e82:
|
Asset size changesSize Auditor did not detect a change in bundle size for any component! Baseline commit: f43aae1493a49053f7fe866a5c56056e73b1d709 (build) |
📊 Bundle size reportUnchanged fixtures
|
Perf Analysis (
|
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| Avatar | mount | 1520 | 1592 | 5000 | |
| Button | mount | 1144 | 1129 | 5000 | |
| FluentProvider | mount | 1873 | 1905 | 5000 | |
| FluentProviderWithTheme | mount | 771 | 769 | 10 | |
| FluentProviderWithTheme | virtual-rerender | 735 | 699 | 10 | |
| FluentProviderWithTheme | virtual-rerender-with-unmount | 763 | 744 | 10 | |
| MakeStyles | mount | 2205 | 2246 | 50000 | |
| SpinButton | mount | 2994 | 3023 | 5000 |
Perf Analysis (
|
| Scenario | Current PR Ticks | Baseline Ticks | Ratio |
|---|---|---|---|
| TreeWith60ListItems.default | 177 | 151 | 1.17:1 |
| PortalMinimalPerf.default | 188 | 166 | 1.13:1 |
| TreeMinimalPerf.default | 840 | 769 | 1.09:1 |
| HeaderMinimalPerf.default | 371 | 343 | 1.08:1 |
| ListWith60ListItems.default | 643 | 593 | 1.08:1 |
| HeaderSlotsPerf.default | 796 | 745 | 1.07:1 |
| CardMinimalPerf.default | 546 | 513 | 1.06:1 |
| CarouselMinimalPerf.default | 462 | 437 | 1.06:1 |
| DialogMinimalPerf.default | 800 | 757 | 1.06:1 |
| ImageMinimalPerf.default | 377 | 354 | 1.06:1 |
| RefMinimalPerf.default | 221 | 208 | 1.06:1 |
| IconMinimalPerf.default | 643 | 608 | 1.06:1 |
| TextMinimalPerf.default | 355 | 336 | 1.06:1 |
| ChatMinimalPerf.default | 739 | 707 | 1.05:1 |
| DropdownManyItemsPerf.default | 666 | 633 | 1.05:1 |
| RosterPerf.default | 2175 | 2077 | 1.05:1 |
| AnimationMinimalPerf.default | 555 | 533 | 1.04:1 |
| SkeletonMinimalPerf.default | 370 | 355 | 1.04:1 |
| BoxMinimalPerf.default | 352 | 341 | 1.03:1 |
| ButtonSlotsPerf.default | 567 | 548 | 1.03:1 |
| DividerMinimalPerf.default | 361 | 349 | 1.03:1 |
| FlexMinimalPerf.default | 279 | 270 | 1.03:1 |
| StatusMinimalPerf.default | 691 | 668 | 1.03:1 |
| TableMinimalPerf.default | 390 | 380 | 1.03:1 |
| ButtonOverridesMissPerf.default | 1351 | 1326 | 1.02:1 |
| ItemLayoutMinimalPerf.default | 1205 | 1186 | 1.02:1 |
| LabelMinimalPerf.default | 372 | 365 | 1.02:1 |
| ListNestedPerf.default | 537 | 525 | 1.02:1 |
| MenuMinimalPerf.default | 859 | 841 | 1.02:1 |
| RadioGroupMinimalPerf.default | 431 | 422 | 1.02:1 |
| ReactionMinimalPerf.default | 389 | 383 | 1.02:1 |
| TextAreaMinimalPerf.default | 477 | 469 | 1.02:1 |
| ButtonMinimalPerf.default | 157 | 156 | 1.01:1 |
| InputMinimalPerf.default | 1141 | 1132 | 1.01:1 |
| LayoutMinimalPerf.default | 355 | 352 | 1.01:1 |
| MenuButtonMinimalPerf.default | 1754 | 1730 | 1.01:1 |
| TableManyItemsPerf.default | 1906 | 1880 | 1.01:1 |
| CustomToolbarPrototype.default | 2711 | 2689 | 1.01:1 |
| ChatWithPopoverPerf.default | 358 | 358 | 1:1 |
| CheckboxMinimalPerf.default | 2131 | 2132 | 1:1 |
| PopupMinimalPerf.default | 642 | 644 | 1:1 |
| ProviderMergeThemesPerf.default | 1309 | 1314 | 1:1 |
| SegmentMinimalPerf.default | 329 | 328 | 1:1 |
| SplitButtonMinimalPerf.default | 4421 | 4426 | 1:1 |
| AccordionMinimalPerf.default | 147 | 148 | 0.99:1 |
| AvatarMinimalPerf.default | 189 | 190 | 0.99:1 |
| GridMinimalPerf.default | 321 | 325 | 0.99:1 |
| ListMinimalPerf.default | 521 | 526 | 0.99:1 |
| LoaderMinimalPerf.default | 666 | 674 | 0.99:1 |
| ProviderMinimalPerf.default | 402 | 407 | 0.99:1 |
| VideoMinimalPerf.default | 727 | 738 | 0.99:1 |
| ChatDuplicateMessagesPerf.default | 285 | 290 | 0.98:1 |
| DropdownMinimalPerf.default | 2674 | 2724 | 0.98:1 |
| SliderMinimalPerf.default | 1600 | 1634 | 0.98:1 |
| TooltipMinimalPerf.default | 2313 | 2355 | 0.98:1 |
| AttachmentSlotsPerf.default | 1106 | 1140 | 0.97:1 |
| DatepickerMinimalPerf.default | 5754 | 5959 | 0.97:1 |
| EmbedMinimalPerf.default | 3630 | 3739 | 0.97:1 |
| FormMinimalPerf.default | 359 | 369 | 0.97:1 |
| ListCommonPerf.default | 628 | 661 | 0.95:1 |
| AttachmentMinimalPerf.default | 136 | 144 | 0.94:1 |
| ToolbarMinimalPerf.default | 911 | 965 | 0.94:1 |
| AlertMinimalPerf.default | 244 | 265 | 0.92:1 |
…d dts after check
Perf Analysis (
|
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| BaseButton | mount | 1202 | 1200 | 5000 | |
| Breadcrumb | mount | 2881 | 2963 | 1000 | |
| Checkbox | mount | 2632 | 2641 | 5000 | |
| CheckboxBase | mount | 2351 | 2296 | 5000 | |
| ChoiceGroup | mount | 4395 | 4361 | 5000 | |
| ComboBox | mount | 1250 | 1250 | 1000 | |
| CommandBar | mount | 9310 | 9316 | 1000 | |
| ContextualMenu | mount | 11282 | 11546 | 1000 | |
| DefaultButton | mount | 1372 | 1387 | 5000 | |
| DetailsRow | mount | 3454 | 3626 | 5000 | |
| DetailsRowFast | mount | 3622 | 3592 | 5000 | |
| DetailsRowNoStyles | mount | 3447 | 3452 | 5000 | |
| Dialog | mount | 3101 | 3119 | 1000 | |
| DocumentCardTitle | mount | 573 | 576 | 1000 | |
| Dropdown | mount | 3238 | 3242 | 5000 | |
| FocusTrapZone | mount | 2006 | 2014 | 5000 | |
| FocusZone | mount | 1983 | 1966 | 5000 | |
| GroupedList | mount | 53085 | 61122 | 2 | |
| GroupedList | virtual-rerender | 24890 | 24730 | 2 | |
| GroupedList | virtual-rerender-with-unmount | 92457 | 93552 | 2 | |
| GroupedListV2 | mount | 558 | 560 | 2 | |
| GroupedListV2 | virtual-rerender | 511 | 529 | 2 | |
| GroupedListV2 | virtual-rerender-with-unmount | 559 | 538 | 2 | |
| IconButton | mount | 1895 | 1914 | 5000 | |
| Label | mount | 725 | 733 | 5000 | |
| Layer | mount | 4213 | 4251 | 5000 | |
| Link | mount | 827 | 753 | 5000 | |
| MenuButton | mount | 1679 | 1590 | 5000 | |
| MessageBar | mount | 2261 | 2188 | 5000 | |
| Nav | mount | 3171 | 3047 | 1000 | |
| OverflowSet | mount | 1362 | 1304 | 5000 | |
| Panel | mount | 2458 | 2394 | 1000 | |
| Persona | mount | 1223 | 1165 | 1000 | |
| Pivot | mount | 1561 | 1539 | 1000 | |
| PrimaryButton | mount | 1519 | 1448 | 5000 | |
| Rating | mount | 7074 | 6949 | 5000 | |
| SearchBox | mount | 1481 | 1525 | 5000 | |
| Shimmer | mount | 2795 | 2764 | 5000 | |
| Slider | mount | 2010 | 2076 | 5000 | |
| SpinButton | mount | 4528 | 4685 | 5000 | |
| Spinner | mount | 814 | 796 | 5000 | |
| SplitButton | mount | 3041 | 3056 | 5000 | |
| Stack | mount | 802 | 860 | 5000 | |
| StackWithIntrinsicChildren | mount | 2332 | 2192 | 5000 | |
| StackWithTextChildren | mount | 4627 | 4587 | 5000 | |
| SwatchColorPicker | mount | 10432 | 10287 | 5000 | |
| TagPicker | mount | 2598 | 2574 | 5000 | |
| TeachingBubble | mount | 97498 | 98137 | 5000 | |
| Text | mount | 715 | 775 | 5000 | |
| TextField | mount | 1566 | 1559 | 5000 | |
| ThemeProvider | mount | 1508 | 1515 | 5000 | |
| ThemeProvider | virtual-rerender | 1066 | 1088 | 5000 | |
| ThemeProvider | virtual-rerender-with-unmount | 2064 | 2057 | 5000 | |
| Toggle | mount | 1115 | 1071 | 5000 | |
| buttonNative | mount | 509 | 556 | 5000 |
…rolluped dts after check
…nd add rolluped dts after check
| /** | ||
| * needs to be explicitly set to `false` so errors propagate to api-extractor | ||
| */ | ||
| skipLibCheck: false, |
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.
will this affect libraries that are not in the monorepo ?
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.
can you elaborate more on your question pls ? what libraries are you referring to.
we need strict library checking turned on when generating rolluped type so we give customers 100% guarantee we ship correct API's. if there is some 3rd party library that has some type violations only we will be affected as that will fail the api generation(pipeline). One example of such a violation is storybook as it uses synthetic default exports. This is handled here https://github.com/microsoft/fluentui/blob/master/scripts/tasks/utils.ts#L22-L27
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.
oh my bad, I misread and thought that we would be type checking 3rd party libraries
| * | ||
| * NOTE: Some v8 packages (font-icons-mdl2) use `preserveConstEnums: false` which clashes with isolateModules - TSC will error | ||
| */ | ||
| isolatedModules: false, |
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.
in v9 packages it would be nice to set this to true, but I guess our build already takes that into account too anyway
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.
it is set to true in all v9 packages already.
isn't the documentation on this property clear ? that it has no effect on declaration files ?
the v8 context:
issue #7110
I was digging more into the icons-mdl2 issue but changing that to emit cons enums would lead to increased js payload size which we dont know what implications that might bring. ATM the problematic api is marked as @deprecated which is ofc not stopping consumers using it.
Removing the deprecated would be breaking change unfortunately.
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, but it would be nice to be consistent with the compiler options we use in our own tsconfigs. I can see the problem with v8 and const enums tho. thx for explaining
| ...(options.tsConfig.files ? { files: options.tsConfig.files } : null), | ||
| ...options.tsConfig, | ||
| compilerOptions: { | ||
| ...options.tsConfig.compilerOptions, |
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.
shouldn't options be spread at the end to override the defaults?
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. those are "The defaults" from tsconfig.lib.json . we override those
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.
ah ok that makes sense
* master: feat(scripts): improve API violation reporting (microsoft#25356) applying package updates fix: Preventing blanket selectors from Fabric component from being applied via new preventBlanketFontInheritance prop (microsoft#25453) feat(react-infobutton): Add initial implementation (microsoft#25247) chore(react-avatar, avatar-context): migrate to new package structure (microsoft#25473) chore(react-portal, portal-compat, portal-compat-context): migrate to new package structure (microsoft#25481) docs(react-infobutton): Adding component's spec (microsoft#25251) fix(screener-build workflow): scope package to build for v9 VR tests to speed up perf (microsoft#25494) chore(vr-tests-v9): Convert Button VR tests to CSF (microsoft#25108)
* fix(script): make copy-compiled task work with packages without 'module' * fix(scripts): make internal api stripping work and add rolluped dts after check
* fix(script): make copy-compiled task work with packages without 'module' * fix(scripts): make internal api stripping work and add rolluped dts after check
Current Behavior
copyCompiledtask dosen't work for node packages ( nomodulefield present )New Behavior
copyCompiledtask works for both web and node packagestsconfig.lib.jsonis used with simple/explicit overridesCLI Example:
yarn workspace @fluentui/react-provider generate-api↓↓↓
yarn workspace @fluentui/react-provider generate-api↓↓↓
Related Issue(s)
Follows #23369