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

Use ts-node for v0 scripts and remove docs alias #12789

Closed
wants to merge 6 commits into from

Conversation

ecraig12345
Copy link
Member

@ecraig12345 ecraig12345 commented Apr 21, 2020

Switch v0 scripts from using @babel/register to ts-node. This is a minor step in converging scripts.

Remove remaining docs aliases from v0 code.

I had to downgrade ts-node to version 7 because version 8 disables caching, and the perf without caching is abysmal (~30s/project for clean!). I also turned off type checking for perf reasons. While downgrading to the version with caching does involve some risk of cache-related issues, I've definitely seen similar issues with @babel/register especially while changing build scripts (unfortunately never pinned down repro steps), so switching to ts-node probably preserves a similar level of risk.

Microsoft Reviewers: Open in CodeFlow

@DustyTheBot
Copy link

DustyTheBot commented Apr 21, 2020

Warnings
⚠️ There are no updates provided to CHANGELOG. Ensure there are no publicly visible changes introduced by this PR.

Generated by 🚫 dangerJS against c1eb5c0

tsNode.register({ dir: __dirname });
tsNode.register({
dir: __dirname,
transpileOnly: true,
Copy link
Member

Choose a reason for hiding this comment

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

So we will not have any benefits from type checks? Am I correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

I can turn on type checks if you want, I was just concerned about regressing build perf.

Copy link
Member Author

Choose a reason for hiding this comment

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

In general I like type checks aside from the perf concerns :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Turned off type checking again due to perf concerns...nearly 30s per project for clean!! 😬

@size-auditor
Copy link

size-auditor bot commented Apr 21, 2020

Asset size changes

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

Baseline commit: 9f4e4375ed8dedc134aecfbfa618a662d512e75b (build)

@msft-github-bot
Copy link
Contributor

msft-github-bot commented Apr 21, 2020

Perf Analysis

No significant results to display.

All results

Scenario Master Ticks PR Ticks Iterations Status
BaseButton 713 739 5000
Checkbox 1418 1347 5000
CheckboxBase 1166 1175 5000
ChoiceGroup 4377 4408 5000
ComboBox 902 856 1000
CommandBar 6663 6778 1000
ContextualMenu 13417 12753 1000
DefaultButton 932 949 5000
DetailsRow 3120 3037 5000
DetailsRow (fast icons) 3035 3079 5000
DetailsRow without styles 2866 2912 5000
Dialog 1376 1339 1000
DocumentCardTitle with truncation 1500 1464 1000
Dropdown 2084 2418 5000
FocusZone 1490 1494 5000
IconButton 1484 1536 5000
Label 272 275 5000
Link 410 401 5000
MenuButton 1250 1261 5000
Nav 2731 2730 1000
Panel 1292 1271 1000
Persona 718 738 1000
Pivot 1197 1174 1000
PrimaryButton 1060 1057 5000
SearchBox 1139 1075 5000
Slider 1329 1315 5000
Spinner 349 346 5000
SplitButton 2743 2706 5000
Stack 421 402 5000
Stack with Intrinsic children 990 977 5000
Stack with Text children 3594 3534 5000
TagPicker 2403 2411 5000
Text 322 315 5000
TextField 1206 1209 5000
Toggle 784 765 5000
button 59 58 5000

Perf Analysis (Fluent)

⚠️ 1 potential perf regressions detected

Potential regressions comparing to master

Scenario Current PR Ticks Baseline Ticks Ratio Regression Analysis
ChatMinimalPerf.default 536 559 0.96:1 analysis
Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🎯 Avatar.Fluent 0.43 0.46 0.93:1 2000 867
🦄 Button.Fluent 0.09 0.17 0.53:1 5000 458
🔧 Checkbox.Fluent 0.61 0.32 1.91:1 1000 610
🔧 Dialog.Fluent 0.31 0.18 1.72:1 5000 1534
🔧 Dropdown.Fluent 3.09 0.41 7.54:1 1000 3088
🔧 Icon.Fluent 0.13 0.05 2.6:1 5000 632
🎯 Image.Fluent 0.07 0.09 0.78:1 5000 352
🔧 Slider.Fluent 1.33 0.36 3.69:1 1000 1328
🔧 Text.Fluent 0.07 0.02 3.5:1 5000 330
🦄 Tooltip.Fluent 0.08 16.19 0:1 5000 418

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
FormMinimalPerf.default 744 663 1.12:1
AttachmentMinimalPerf.default 134 123 1.09:1
LayoutMinimalPerf.default 503 460 1.09:1
ToolbarMinimalPerf.default 890 822 1.08:1
LabelMinimalPerf.default 382 356 1.07:1
ChatWithPopoverPerf.default 574 542 1.06:1
PortalMinimalPerf.default 269 254 1.06:1
EmbedMinimalPerf.default 4058 3858 1.05:1
TextAreaMinimalPerf.default 2616 2499 1.05:1
TreeMinimalPerf.default 1090 1042 1.05:1
Tooltip.Fluent 418 397 1.05:1
AttachmentSlotsPerf.default 1054 1013 1.04:1
ButtonSlotsPerf.default 535 515 1.04:1
InputMinimalPerf.default 902 871 1.04:1
RefMinimalPerf.default 193 186 1.04:1
TableMinimalPerf.default 496 476 1.04:1
Dropdown.Fluent 3088 2967 1.04:1
Slider.Fluent 1328 1279 1.04:1
Text.Fluent 330 317 1.04:1
AlertMinimalPerf.default 269 261 1.03:1
AnimationMinimalPerf.default 581 565 1.03:1
AvatarMinimalPerf.default 468 454 1.03:1
DividerMinimalPerf.default 653 636 1.03:1
ItemLayoutMinimalPerf.default 1475 1426 1.03:1
ListMinimalPerf.default 444 433 1.03:1
PopupMinimalPerf.default 235 229 1.03:1
TooltipMinimalPerf.default 652 630 1.03:1
Button.Fluent 458 444 1.03:1
BoxMinimalPerf.default 301 294 1.02:1
CardMinimalPerf.default 535 522 1.02:1
CarouselMinimalPerf.default 554 545 1.02:1
CheckboxMinimalPerf.default 2701 2655 1.02:1
HeaderSlotsPerf.default 1334 1314 1.02:1
ListCommonPerf.default 927 906 1.02:1
SplitButtonMinimalPerf.default 3397 3323 1.02:1
StatusMinimalPerf.default 622 607 1.02:1
Checkbox.Fluent 610 599 1.02:1
Image.Fluent 352 345 1.02:1
ListNestedPerf.default 825 813 1.01:1
RadioGroupMinimalPerf.default 509 504 1.01:1
TextMinimalPerf.default 299 296 1.01:1
DropdownMinimalPerf.default 3038 3029 1:1
HeaderMinimalPerf.default 439 440 1:1
CustomToolbarPrototype.default 3303 3316 1:1
DialogMinimalPerf.default 1541 1553 0.99:1
HierarchicalTreeMinimalPerf.default 882 893 0.99:1
ReactionMinimalPerf.default 1680 1691 0.99:1
IconMinimalPerf.default 597 603 0.99:1
TreeWith60ListItems.default 198 199 0.99:1
Avatar.Fluent 867 876 0.99:1
Icon.Fluent 632 638 0.99:1
ListWith60ListItems.default 1048 1066 0.98:1
ProviderMergeThemesPerf.default 1502 1525 0.98:1
VideoMinimalPerf.default 554 567 0.98:1
ButtonMinimalPerf.default 142 146 0.97:1
ChatDuplicateMessagesPerf.default 355 365 0.97:1
DropdownManyItemsPerf.default 1222 1258 0.97:1
FlexMinimalPerf.default 264 271 0.97:1
LoaderMinimalPerf.default 723 749 0.97:1
Dialog.Fluent 1534 1585 0.97:1
GridMinimalPerf.default 584 608 0.96:1
MenuButtonMinimalPerf.default 1388 1442 0.96:1
ProviderMinimalPerf.default 601 629 0.96:1
SegmentMinimalPerf.default 821 853 0.96:1
SliderMinimalPerf.default 1258 1323 0.95:1
MenuMinimalPerf.default 1576 1703 0.93:1
AccordionMinimalPerf.default 169 184 0.92:1
ImageMinimalPerf.default 336 373 0.9:1

@ecraig12345
Copy link
Member Author

ecraig12345 commented Apr 22, 2020

Note in case anyone else wants to try it, I've been testing perf with this absurdly long command intended to target only the projects still using gulp:
lerna run clean --scope '@fluentui/!(react|examples|digest|perf-test|playground|react-theming|react-compose|react-focus|react-icons|keyboard-key)'

Running clean is a good way to test transpile perf (the main concern here) since the length of the task itself is minimal and fairly consistent across runs (at least after the first run).

Note that both @babel/register and ts-node 7 use caching (so the first run will also be slower for that reason). The babel cache is at <pkg>/node_modules/.cache/@babel/register. The ts-node cache is at <os tmpdir>/ts-node-xxxx.

@ecraig12345
Copy link
Member Author

There may also be issues with how ts-node handles output, at least the way that gulp does it. With gulp-based tasks, it shows all the output at once rather than as it comes. I haven't noticed this problem in the just-based tasks which also use ts-node, so not sure what's going on there.

@ecraig12345 ecraig12345 force-pushed the ts-node-v0 branch 2 times, most recently from 7c6cd49 to c1eb5c0 Compare April 22, 2020 05:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants