chore: enable isolateModules in all v8 packages #25774
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Hotell
commented
Nov 24, 2022
| * Types in this file provide const enums replacement that will compile with isolatedModules | ||
| */ | ||
|
|
||
| import type { ApiItemKind as _ApiItemKind } from '@microsoft/api-extractor-model'; |
Contributor
Author
There was a problem hiding this comment.
unfortunately those packages heavily rely on const enums, so to be able to have isolatedModules this is the only workaround
Hotell
commented
Nov 24, 2022
| @@ -1,17 +1,24 @@ | |||
| export { IStyle, IStyleBase, IStyleBaseArray } from './IStyle'; | |||
| export type { IStyle, IStyleBase, IStyleBaseArray } from './IStyle'; | |||
Contributor
Author
There was a problem hiding this comment.
properly enforced type exports - thanks to isolateModules
Hotell
added
Fluent UI react (v8)
Collaborator
📊 Bundle size reportUnchanged fixtures
|
Collaborator
Perf Analysis (
|
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| Avatar | mount | 1265 | 1298 | 5000 | |
| Button | mount | 914 | 935 | 5000 | |
| FluentProvider | mount | 1500 | 1502 | 5000 | |
| FluentProviderWithTheme | mount | 570 | 584 | 10 | |
| FluentProviderWithTheme | virtual-rerender | 540 | 550 | 10 | |
| FluentProviderWithTheme | virtual-rerender-with-unmount | 573 | 579 | 10 | |
| MakeStyles | mount | 1952 | 1977 | 50000 | |
| SpinButton | mount | 2347 | 2298 | 5000 |
Collaborator
🕵 fluentuiv8 Open the Visual Regressions report to inspect the 7 screenshots✅ There was 0 screenshots added, 4 screenshots removed, 1032 screenshots unchanged, 0 screenshots with different dimensions and 3 screenshots with visible difference. unknown 7 screenshots
|
Collaborator
Perf Analysis (
|
| Scenario | Current PR Ticks | Baseline Ticks | Ratio |
|---|---|---|---|
| HeaderSlotsPerf.default | 853 | 749 | 1.14:1 |
| BoxMinimalPerf.default | 362 | 321 | 1.13:1 |
| ListWith60ListItems.default | 664 | 604 | 1.1:1 |
| ReactionMinimalPerf.default | 391 | 359 | 1.09:1 |
| TextAreaMinimalPerf.default | 496 | 461 | 1.08:1 |
| AnimationMinimalPerf.default | 548 | 514 | 1.07:1 |
| SkeletonMinimalPerf.default | 348 | 326 | 1.07:1 |
| ProviderMinimalPerf.default | 436 | 411 | 1.06:1 |
| RadioGroupMinimalPerf.default | 456 | 431 | 1.06:1 |
| FormMinimalPerf.default | 392 | 374 | 1.05:1 |
| StatusMinimalPerf.default | 716 | 685 | 1.05:1 |
| ChatWithPopoverPerf.default | 366 | 352 | 1.04:1 |
| FlexMinimalPerf.default | 305 | 292 | 1.04:1 |
| ProviderMergeThemesPerf.default | 1342 | 1285 | 1.04:1 |
| InputMinimalPerf.default | 1206 | 1171 | 1.03:1 |
| RosterPerf.default | 2226 | 2160 | 1.03:1 |
| PopupMinimalPerf.default | 682 | 659 | 1.03:1 |
| SegmentMinimalPerf.default | 366 | 355 | 1.03:1 |
| IconMinimalPerf.default | 674 | 652 | 1.03:1 |
| DatepickerMinimalPerf.default | 6223 | 6083 | 1.02:1 |
| DropdownMinimalPerf.default | 2730 | 2679 | 1.02:1 |
| SplitButtonMinimalPerf.default | 4577 | 4468 | 1.02:1 |
| ToolbarMinimalPerf.default | 968 | 947 | 1.02:1 |
| AttachmentSlotsPerf.default | 1104 | 1092 | 1.01:1 |
| CheckboxMinimalPerf.default | 2065 | 2050 | 1.01:1 |
| GridMinimalPerf.default | 360 | 355 | 1.01:1 |
| SliderMinimalPerf.default | 1593 | 1582 | 1.01:1 |
| CustomToolbarPrototype.default | 2799 | 2765 | 1.01:1 |
| AvatarMinimalPerf.default | 199 | 200 | 1:1 |
| CarouselMinimalPerf.default | 469 | 470 | 1:1 |
| EmbedMinimalPerf.default | 3691 | 3690 | 1:1 |
| ItemLayoutMinimalPerf.default | 1241 | 1235 | 1:1 |
| RefMinimalPerf.default | 234 | 234 | 1:1 |
| TooltipMinimalPerf.default | 2347 | 2346 | 1:1 |
| ButtonMinimalPerf.default | 171 | 173 | 0.99:1 |
| ButtonOverridesMissPerf.default | 1314 | 1328 | 0.99:1 |
| ChatDuplicateMessagesPerf.default | 266 | 268 | 0.99:1 |
| ChatMinimalPerf.default | 714 | 723 | 0.99:1 |
| LabelMinimalPerf.default | 382 | 385 | 0.99:1 |
| LayoutMinimalPerf.default | 343 | 345 | 0.99:1 |
| TableManyItemsPerf.default | 1896 | 1908 | 0.99:1 |
| TreeMinimalPerf.default | 833 | 838 | 0.99:1 |
| VideoMinimalPerf.default | 746 | 750 | 0.99:1 |
| ButtonSlotsPerf.default | 540 | 549 | 0.98:1 |
| ListNestedPerf.default | 557 | 570 | 0.98:1 |
| AttachmentMinimalPerf.default | 136 | 140 | 0.97:1 |
| DialogMinimalPerf.default | 775 | 798 | 0.97:1 |
| DividerMinimalPerf.default | 349 | 359 | 0.97:1 |
| ListMinimalPerf.default | 497 | 512 | 0.97:1 |
| DropdownManyItemsPerf.default | 660 | 691 | 0.96:1 |
| ListCommonPerf.default | 659 | 689 | 0.96:1 |
| TableMinimalPerf.default | 400 | 417 | 0.96:1 |
| AccordionMinimalPerf.default | 138 | 146 | 0.95:1 |
| AlertMinimalPerf.default | 257 | 271 | 0.95:1 |
| TextMinimalPerf.default | 348 | 368 | 0.95:1 |
| LoaderMinimalPerf.default | 351 | 375 | 0.94:1 |
| MenuMinimalPerf.default | 848 | 898 | 0.94:1 |
| MenuButtonMinimalPerf.default | 1687 | 1795 | 0.94:1 |
| ImageMinimalPerf.default | 366 | 394 | 0.93:1 |
| CardMinimalPerf.default | 515 | 567 | 0.91:1 |
| PortalMinimalPerf.default | 158 | 178 | 0.89:1 |
| HeaderMinimalPerf.default | 338 | 384 | 0.88:1 |
| TreeWith60ListItems.default | 153 | 186 | 0.82:1 |
Collaborator
🕵 fluentuiv9 Open the Visual Regressions report to inspect the 2 screenshots✅ There was 0 screenshots added, 0 screenshots removed, 1752 screenshots unchanged, 0 screenshots with different dimensions and 2 screenshots with visible difference. unknown 2 screenshots
|
Hotell
commented
Nov 24, 2022
| // eslint-disable-next-line deprecation/deprecation | ||
| export { IconNames, IconNamesInput } from './IconNames'; | ||
| /* eslint-disable deprecation/deprecation */ | ||
| export type { IconNamesInput } from './IconNames'; |
Contributor
Author
There was a problem hiding this comment.
although this package is the only one in monorepo that cannot use isolateModules because const enum is used ( it would increase customer bundle size if they use typescript for compilation. if customer is not using typescript this wouldn;t be breaking change) we still get warnings from swc/babel because incorrectly re-exported type.
Asset size changesSize Auditor did not detect a change in bundle size for any component! Baseline commit: 076ac58a921376c6ee488f10e610dc05095acbf3 (build) |
|
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 6674c82:
|
Collaborator
Perf Analysis (
|
| Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
|---|---|---|---|---|---|
| BaseButton | mount | 1197 | 1206 | 5000 | |
| Breadcrumb | mount | 2784 | 2873 | 1000 | |
| Checkbox | mount | 2662 | 2601 | 5000 | |
| CheckboxBase | mount | 2378 | 2374 | 5000 | |
| ChoiceGroup | mount | 4304 | 4264 | 5000 | |
| ComboBox | mount | 1194 | 1156 | 1000 | |
| CommandBar | mount | 9206 | 9296 | 1000 | |
| ContextualMenu | mount | 10162 | 10146 | 1000 | |
| DefaultButton | mount | 1356 | 1367 | 5000 | |
| DetailsRow | mount | 3411 | 3399 | 5000 | |
| DetailsRowFast | mount | 3395 | 3405 | 5000 | |
| DetailsRowNoStyles | mount | 3230 | 3255 | 5000 | |
| Dialog | mount | 2980 | 2948 | 1000 | |
| DocumentCardTitle | mount | 583 | 579 | 1000 | |
| Dropdown | mount | 3145 | 3168 | 5000 | |
| FocusTrapZone | mount | 1955 | 1960 | 5000 | |
| FocusZone | mount | 1933 | 1941 | 5000 | |
| GroupedList | mount | 1836 | 2064 | 2 | |
| GroupedList | virtual-rerender | 1114 | 1103 | 2 | |
| GroupedList | virtual-rerender-with-unmount | 1609 | 1625 | 2 | |
| GroupedListV2 | mount | 556 | 576 | 2 | |
| GroupedListV2 | virtual-rerender | 548 | 565 | 2 | |
| GroupedListV2 | virtual-rerender-with-unmount | 563 | 568 | 2 | |
| IconButton | mount | 1801 | 1797 | 5000 | |
| Label | mount | 753 | 770 | 5000 | |
| Layer | mount | 4170 | 4232 | 5000 | |
| Link | mount | 847 | 831 | 5000 | |
| MenuButton | mount | 1600 | 1620 | 5000 | |
| MessageBar | mount | 2317 | 2341 | 5000 | |
| Nav | mount | 3045 | 3099 | 1000 | |
| OverflowSet | mount | 1412 | 1408 | 5000 | |
| Panel | mount | 2565 | 2477 | 1000 | |
| Persona | mount | 1254 | 1307 | 1000 | |
| Pivot | mount | 1497 | 1634 | 1000 | |
| PrimaryButton | mount | 1491 | 1504 | 5000 | |
| Rating | mount | 7005 | 7020 | 5000 | |
| SearchBox | mount | 1508 | 1524 | 5000 | |
| Shimmer | mount | 2919 | 2918 | 5000 | |
| Slider | mount | 2103 | 2133 | 5000 | |
| SpinButton | mount | 4340 | 4285 | 5000 | |
| Spinner | mount | 841 | 821 | 5000 | |
| SplitButton | mount | 2863 | 2857 | 5000 | |
| Stack | mount | 864 | 861 | 5000 | |
| StackWithIntrinsicChildren | mount | 2352 | 2332 | 5000 | |
| StackWithTextChildren | mount | 5050 | 5044 | 5000 | |
| SwatchColorPicker | mount | 9612 | 9467 | 5000 | |
| TagPicker | mount | 2325 | 2339 | 5000 | |
| TeachingBubble | mount | 76172 | 76002 | 5000 | |
| Text | mount | 833 | 820 | 5000 | |
| TextField | mount | 1553 | 1563 | 5000 | |
| ThemeProvider | mount | 1446 | 1449 | 5000 | |
| ThemeProvider | virtual-rerender | 1141 | 1136 | 5000 | |
| ThemeProvider | virtual-rerender-with-unmount | 1992 | 2010 | 5000 | |
| Toggle | mount | 1146 | 1135 | 5000 | |
| buttonNative | mount | 534 | 535 | 5000 |
Hotell
changed the title
Hotell
requested review from
a team,
Jahnp,
bigbadcapers and
dzearing
as code owners
Hotell
requested review from
a team,
Jacqueline-ms,
khmakoto and
robtaft-ms
as code owners
ling1726
approved these changes
Nov 28, 2022
spmonahan
approved these changes
Nov 28, 2022
khmakoto
approved these changes
Nov 28, 2022
micahgodbolt
approved these changes
Nov 28, 2022
tudorpopams
approved these changes
Nov 29, 2022
Hotell
force-pushed
the
hotell/v8/fix-isolate-modules
branch
from
ad5a13f to
6674c82
Compare
AtishayMsft
approved these changes
Nov 30, 2022
Contributor
Author
|
/azp run |
|
Azure Pipelines successfully started running 4 pipeline(s). |
marcosmoura
added a commit
to marcosmoura/fluentui
that referenced
this pull request
Dec 1, 2022
* master: BREAKING: `useTable` renamed to `useTableFeatures` (microsoft#25797) chore: add retries for navigation in ssr-tests-v9 (microsoft#25844) fix: Cell actions should have correct background when row focused within (microsoft#25790) applying package updates Disable 3 Avatar Converged active stories (microsoft#25765) chore: introduce TS path aliases for improved DX in v8 (microsoft#25778) chore: prepare release react-northstar 0.65.0 (microsoft#25446) refactor(scripts): encapsulate v0 and v8 tooling within its domain boundaries (microsoft#25738) Support single point in area chart (microsoft#25842) chore: enable isolateModules in all v8 packages (microsoft#25774) chore: refactor styles for Button (microsoft#25216) feat: Improve docs for `DataGrid`, export as unstable (microsoft#25805) applying package updates fix: Allow data-selection-disabled to be respected by DetailsRow (microsoft#25836) docs(rfcs): Update recipes rfc with chosen option and add more details (microsoft#25823) chore(react-textarea): migrate to new package structure (microsoft#25820) chore(react-switch): migrate to new package structure (microsoft#25819) fix(react-avatar): AvatarGroup's pie layout places inline items correctly in rtl (microsoft#25822) chore: add few improvements to toolbar stories (microsoft#25635)
Hotell
added a commit
to Hotell/fluentui
that referenced
this pull request
Feb 9, 2023
* chore: enable isolateModules in all packages * generate changefiles * feat: remove obsolete useTheme knob from storybook package that were causing isolateModules errors
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
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.
Previous Behavior
not all v8 and v8 related packages have enabled
isolatedModuleswhich leads to compiler errors/warnings when esbuild/swc/babel is being used.not following isolatedModules may also cause worse tree-shake-ability on consumers side and issues when migrating to new TS versions in the future.
New Behavior
All v8 and v8 related packages have enabled
isolatedModulestypeimports applied withinmerge-styleswhich is a direct dependency of fluentui/react (the main culpirt of compiler errors when using v8)@fluentui/font-icons-mdl2remains the only package in our repo withoutisolatedModulesRelated Issue(s)