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(tools): preserve types and tags when running migration on already migrated project #18554

Conversation

Hotell
Copy link
Contributor

@Hotell Hotell commented Jun 14, 2021

…ion generator

Pull request checklist

Description of changes

  • add platform:web to nx.json tags as new organisation classifier (will be used in the future with eslint rule to enforce boundaries)
  • fix: migration will preserve additional types in package tsconfig.json if present
  • tweaks TSconfig interface to match real editor config instead of processed options within tsc compiler

TODO:

  • add platform tags to nx.json
  • [ ] properly provide path source in tsconfig.base.json based on real file system (eg node packages might be written in JS thus index.js needs to be provided -> not applicable, implement in separate migration generator

Focus areas to test

(optional)

@size-auditor
Copy link

size-auditor bot commented Jun 14, 2021

Asset size changes

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

Baseline commit: 6abe9b4670d0ff9cef0ee6fe09bf62db01d8c675 (build)

@fabricteam
Copy link
Collaborator

fabricteam commented Jun 14, 2021

Perf Analysis (@fluentui/react)

Scenario Render type Master Ticks PR Ticks Iterations Status
Panel mount 2047 1380 1000 Possible regression
All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 823 799 5000
BaseButton mount 896 923 5000
Breadcrumb mount 2710 2661 1000
ButtonNext mount 527 525 5000
Checkbox mount 1525 1544 5000
CheckboxBase mount 1309 1331 5000
ChoiceGroup mount 4749 4747 5000
ComboBox mount 1039 986 1000
CommandBar mount 10202 10252 1000
ContextualMenu mount 6405 6275 1000
DefaultButton mount 1145 1111 5000
DetailsRow mount 3748 3779 5000
DetailsRowFast mount 3746 3762 5000
DetailsRowNoStyles mount 3543 3496 5000
Dialog mount 2192 2171 1000
DocumentCardTitle mount 149 133 1000
Dropdown mount 3232 3219 5000
FocusTrapZone mount 1812 1805 5000
FocusZone mount 1812 1841 5000
IconButton mount 1711 1717 5000
Label mount 326 341 5000
Layer mount 1792 1759 5000
Link mount 464 476 5000
MakeStyles mount 1810 1846 50000
MenuButton mount 1457 1433 5000
MessageBar mount 2038 2061 5000
Nav mount 3179 3243 1000
OverflowSet mount 1031 1032 5000
Panel mount 2047 1380 1000 Possible regression
Persona mount 837 818 1000
Pivot mount 1407 1410 1000
PrimaryButton mount 1273 1278 5000
Rating mount 7526 7633 5000
SearchBox mount 1299 1314 5000
Shimmer mount 2518 2545 5000
Slider mount 1984 1963 5000
SpinButton mount 4929 4957 5000
Spinner mount 425 412 5000
SplitButton mount 3152 3186 5000
Stack mount 503 501 5000
StackWithIntrinsicChildren mount 1512 1519 5000
StackWithTextChildren mount 4474 4516 5000
SwatchColorPicker mount 10191 10206 5000
Tabs mount 1387 1393 1000
TagPicker mount 2431 2421 5000
TeachingBubble mount 12057 12025 5000
Text mount 433 430 5000
TextField mount 1381 1360 5000
ThemeProvider mount 1192 1253 5000
ThemeProvider virtual-rerender 628 606 5000
ThemeProviderNext mount 7172 7164 5000
Toggle mount 813 808 5000
buttonNative mount 115 118 5000

Perf Analysis (@fluentui/react-northstar)

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
FlexMinimalPerf.default 297 271 1.1:1
AttachmentMinimalPerf.default 157 146 1.08:1
AvatarMinimalPerf.default 201 186 1.08:1
ButtonMinimalPerf.default 176 163 1.08:1
ChatDuplicateMessagesPerf.default 294 274 1.07:1
SegmentMinimalPerf.default 351 328 1.07:1
BoxMinimalPerf.default 359 339 1.06:1
LoaderMinimalPerf.default 714 673 1.06:1
HeaderMinimalPerf.default 360 342 1.05:1
LabelMinimalPerf.default 392 373 1.05:1
ListWith60ListItems.default 647 618 1.05:1
TreeMinimalPerf.default 837 798 1.05:1
ChatWithPopoverPerf.default 374 360 1.04:1
PortalMinimalPerf.default 185 178 1.04:1
RefMinimalPerf.default 248 238 1.04:1
AccordionMinimalPerf.default 151 146 1.03:1
GridMinimalPerf.default 343 334 1.03:1
ListCommonPerf.default 631 615 1.03:1
RadioGroupMinimalPerf.default 448 434 1.03:1
SliderMinimalPerf.default 1560 1514 1.03:1
StatusMinimalPerf.default 671 652 1.03:1
TableMinimalPerf.default 403 390 1.03:1
ToolbarMinimalPerf.default 953 929 1.03:1
AttachmentSlotsPerf.default 1145 1125 1.02:1
DropdownManyItemsPerf.default 690 679 1.02:1
InputMinimalPerf.default 1254 1234 1.02:1
MenuMinimalPerf.default 866 853 1.02:1
ProviderMinimalPerf.default 975 953 1.02:1
TableManyItemsPerf.default 1918 1886 1.02:1
TooltipMinimalPerf.default 982 967 1.02:1
ButtonOverridesMissPerf.default 1701 1681 1.01:1
ChatMinimalPerf.default 621 613 1.01:1
DatepickerMinimalPerf.default 5385 5343 1.01:1
DropdownMinimalPerf.default 3078 3036 1.01:1
FormMinimalPerf.default 390 386 1.01:1
HeaderSlotsPerf.default 754 743 1.01:1
ItemLayoutMinimalPerf.default 1245 1234 1.01:1
MenuButtonMinimalPerf.default 1593 1579 1.01:1
RosterPerf.default 1198 1181 1.01:1
SkeletonMinimalPerf.default 350 346 1.01:1
SplitButtonMinimalPerf.default 3655 3634 1.01:1
VideoMinimalPerf.default 636 632 1.01:1
ButtonSlotsPerf.default 543 542 1:1
DialogMinimalPerf.default 747 747 1:1
EmbedMinimalPerf.default 4059 4063 1:1
ListNestedPerf.default 546 545 1:1
CustomToolbarPrototype.default 3760 3761 1:1
AlertMinimalPerf.default 270 272 0.99:1
CardMinimalPerf.default 544 548 0.99:1
CarouselMinimalPerf.default 466 471 0.99:1
DividerMinimalPerf.default 365 368 0.99:1
ImageMinimalPerf.default 362 364 0.99:1
ListMinimalPerf.default 511 515 0.99:1
PopupMinimalPerf.default 564 568 0.99:1
ProviderMergeThemesPerf.default 1662 1687 0.99:1
TextAreaMinimalPerf.default 490 493 0.99:1
TreeWith60ListItems.default 169 171 0.99:1
AnimationMinimalPerf.default 418 426 0.98:1
CheckboxMinimalPerf.default 2711 2771 0.98:1
LayoutMinimalPerf.default 359 365 0.98:1
IconMinimalPerf.default 600 611 0.98:1
TextMinimalPerf.default 340 347 0.98:1
ReactionMinimalPerf.default 374 386 0.97:1

@Hotell Hotell marked this pull request as ready for review June 16, 2021 09:56
@Hotell Hotell requested a review from a team as a code owner June 16, 2021 09:56
@@ -1,6 +1,22 @@
type RemoveIndex<T extends Record<string, unknown>> = Pick<T, KnownKeys<T>>;
type KnownKeys<T> = {
Copy link
Member

Choose a reason for hiding this comment

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

I don't have a problem with what the script is doing, but these types look pretty hard to maintain :/

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 is standard mapped type if one wants get rid of index signatures. Ideally we should have a package that implements this utilities. It's definitely overkill for this migration script but because CompilerOptions is implemented how it is, this is radically easier effort than having whole compiler options written by hand and keep synced with actuall TS version

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add some JSDoc for explanation what it does

@Hotell Hotell mentioned this pull request Jun 16, 2021
2 tasks
@Hotell Hotell changed the title feat(tools): differentiate between node and browser package in migrat… fix(tools): preserve types and tags when running migration on already migrated project Jun 16, 2021
@Hotell Hotell merged commit bbbed62 into microsoft:master Jun 16, 2021
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.

4 participants