Skip to content

chore: apply single version policy on jest#16950

Merged
Hotell merged 3 commits intomicrosoft:masterfrom
Hotell:hotell/16889/jest-svp
Feb 17, 2021
Merged

chore: apply single version policy on jest#16950
Hotell merged 3 commits intomicrosoft:masterfrom
Hotell:hotell/16889/jest-svp

Conversation

@Hotell
Copy link
Contributor

@Hotell Hotell commented Feb 11, 2021 •
edited
Loading

Pull request checklist

Description of changes

  • apply single version policy for jest and friends
  • set explicit versions

Focus areas to test

@DustyTheBot
Copy link

DustyTheBot commented Feb 11, 2021 •
edited
Loading

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

Generated by 🚫 dangerJS against ba3f4c1

@codesandbox-ci
Copy link

codesandbox-ci bot commented Feb 11, 2021 •
edited
Loading

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 ba3f4c1:

Sandbox Source
Fluent UI Button Configuration
codesandbox-react-template Configuration
codesandbox-react-northstar-template Configuration

@size-auditor
Copy link

size-auditor bot commented Feb 11, 2021 •
edited
Loading

Asset size changes

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

Baseline commit: 01dd9726d4c6e3e19fc2891275b593cc9a116d44 (build)

@Hotell Hotell mentioned this pull request Feb 11, 2021
Closed
13 tasks
@fabricteam
Copy link
Collaborator

fabricteam commented Feb 11, 2021 •
edited
Loading

Perf Analysis

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 801 810 5000
BaseButton mount 877 880 5000
Breadcrumb mount 43553 43601 5000
ButtonNext mount 682 656 5000
Checkbox mount 1476 1490 5000
CheckboxBase mount 1248 1225 5000
ChoiceGroup mount 4661 4662 5000
ComboBox mount 949 948 1000
CommandBar mount 10047 10082 1000
ContextualMenu mount 6054 6084 1000
DefaultButton mount 1121 1122 5000
DetailsRow mount 3512 3460 5000
DetailsRowFast mount 3635 3521 5000
DetailsRowNoStyles mount 3348 3438 5000
Dialog mount 1440 1444 1000
DocumentCardTitle mount 1848 1805 1000
Dropdown mount 3268 3274 5000
FocusTrapZone mount 1751 1768 5000
FocusZone mount 1826 1790 5000
IconButton mount 1739 1778 5000
Label mount 323 322 5000
Layer mount 1727 1762 5000
Link mount 450 467 5000
MakeStyles mount 1962 1933 50000
MenuButton mount 1451 1449 5000
MessageBar mount 2011 1950 5000
Nav mount 3206 3194 1000
OverflowSet mount 1011 1028 5000
Panel mount 1424 1401 1000
Persona mount 809 821 1000
Pivot mount 1366 1370 1000
PrimaryButton mount 1275 1274 5000
Rating mount 7402 7412 5000
SearchBox mount 1277 1282 5000
Shimmer mount 2510 2536 5000
Slider mount 1896 1899 5000
SpinButton mount 4923 4849 5000
Spinner mount 401 404 5000
SplitButton mount 3085 3123 5000
Stack mount 500 492 5000
StackWithIntrinsicChildren mount 1518 1558 5000
StackWithTextChildren mount 4405 4416 5000
SwatchColorPicker mount 10178 10087 5000
Tabs mount 1367 1392 1000
TagPicker mount 2810 2730 5000
TeachingBubble mount 11446 11523 5000
Text mount 403 409 5000
TextField mount 1368 1309 5000
ThemeProvider mount 1187 1164 5000
ThemeProvider virtual-rerender 595 593 5000
ThemeProviderNext mount 1988 1982 5000
Toggle mount 799 802 5000
buttonNative mount 104 108 5000

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🦄 Avatar.Fluent 0.18 0.48 0.38:1 2000 351
🦄 Button.Fluent 0.12 0.2 0.6:1 5000 591
🔧 Checkbox.Fluent 0.65 0.33 1.97:1 1000 647
🎯 Dialog.Fluent 0.16 0.21 0.76:1 5000 806
🔧 Dropdown.Fluent 3.08 0.4 7.7:1 1000 3076
🔧 Icon.Fluent 0.14 0.06 2.33:1 5000 690
🦄 Image.Fluent 0.08 0.13 0.62:1 5000 413
🔧 Slider.Fluent 1.63 0.45 3.62:1 1000 1625
🔧 Text.Fluent 0.07 0.03 2.33:1 5000 373
🦄 Tooltip.Fluent 0.12 0.9 0.13:1 5000 583

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
ReactionMinimalPerf.default 434 408 1.06:1
ChatDuplicateMessagesPerf.default 380 361 1.05:1
TextAreaMinimalPerf.default 502 479 1.05:1
AnimationMinimalPerf.default 414 397 1.04:1
CardMinimalPerf.default 592 571 1.04:1
DividerMinimalPerf.default 399 385 1.04:1
LayoutMinimalPerf.default 452 433 1.04:1
ListMinimalPerf.default 544 524 1.04:1
ListWith60ListItems.default 660 636 1.04:1
LoaderMinimalPerf.default 766 740 1.04:1
MenuMinimalPerf.default 916 879 1.04:1
RefMinimalPerf.default 262 253 1.04:1
SkeletonMinimalPerf.default 391 376 1.04:1
TableMinimalPerf.default 432 416 1.04:1
AvatarMinimalPerf.default 211 204 1.03:1
DialogMinimalPerf.default 816 789 1.03:1
FlexMinimalPerf.default 325 315 1.03:1
GridMinimalPerf.default 373 361 1.03:1
HeaderMinimalPerf.default 392 382 1.03:1
PopupMinimalPerf.default 732 714 1.03:1
PortalMinimalPerf.default 178 172 1.03:1
TableManyItemsPerf.default 2079 2019 1.03:1
Tooltip.Fluent 583 566 1.03:1
AttachmentMinimalPerf.default 173 170 1.02:1
AttachmentSlotsPerf.default 1240 1219 1.02:1
BoxMinimalPerf.default 384 375 1.02:1
CarouselMinimalPerf.default 497 487 1.02:1
ChatWithPopoverPerf.default 457 448 1.02:1
DropdownManyItemsPerf.default 743 732 1.02:1
ItemLayoutMinimalPerf.default 1247 1226 1.02:1
ListCommonPerf.default 678 662 1.02:1
MenuButtonMinimalPerf.default 1600 1567 1.02:1
ProviderMinimalPerf.default 1000 977 1.02:1
TextMinimalPerf.default 379 371 1.02:1
VideoMinimalPerf.default 662 649 1.02:1
Avatar.Fluent 351 345 1.02:1
Button.Fluent 591 579 1.02:1
Image.Fluent 413 405 1.02:1
Slider.Fluent 1625 1587 1.02:1
AlertMinimalPerf.default 312 309 1.01:1
ButtonMinimalPerf.default 185 183 1.01:1
ButtonOverridesMissPerf.default 1694 1684 1.01:1
ButtonUseCssPerf.default 829 820 1.01:1
ChatMinimalPerf.default 634 627 1.01:1
CheckboxMinimalPerf.default 2902 2885 1.01:1
DatepickerMinimalPerf.default 47263 46977 1.01:1
FormMinimalPerf.default 430 427 1.01:1
ListNestedPerf.default 597 590 1.01:1
SegmentMinimalPerf.default 377 374 1.01:1
SliderMinimalPerf.default 1625 1608 1.01:1
StatusMinimalPerf.default 741 734 1.01:1
IconMinimalPerf.default 707 703 1.01:1
TooltipMinimalPerf.default 831 819 1.01:1
TreeMinimalPerf.default 817 806 1.01:1
Checkbox.Fluent 647 639 1.01:1
ButtonUseCssNestingPerf.default 1090 1088 1:1
EmbedMinimalPerf.default 4285 4275 1:1
ImageMinimalPerf.default 402 400 1:1
InputMinimalPerf.default 1323 1328 1:1
ProviderMergeThemesPerf.default 1634 1633 1:1
SplitButtonMinimalPerf.default 3766 3774 1:1
ToolbarMinimalPerf.default 988 988 1:1
AccordionMinimalPerf.default 170 172 0.99:1
DropdownMinimalPerf.default 3097 3135 0.99:1
HeaderSlotsPerf.default 794 799 0.99:1
LabelMinimalPerf.default 430 433 0.99:1
RadioGroupMinimalPerf.default 467 472 0.99:1
CustomToolbarPrototype.default 3783 3821 0.99:1
Dialog.Fluent 806 812 0.99:1
Dropdown.Fluent 3076 3109 0.99:1
Icon.Fluent 690 695 0.99:1
ButtonSlotsPerf.default 577 602 0.96:1
RosterPerf.default 1158 1208 0.96:1
Text.Fluent 373 390 0.96:1
TreeWith60ListItems.default 177 188 0.94:1

@ecraig12345
Copy link
Member

ecraig12345 commented Feb 12, 2021 •
edited
Loading

LGTM so far. I was looking at the docs for import/no-extraneous-dependencies and it looks like you can actually set it up to allow checking multiple package.json files. Unfortunately it can't be set up to allow multiple package.jsons only for devDependencies, so one option would be to change the main rule config to this:

    'import/no-extraneous-dependencies': ['error', { devDependencies: false }], // don't allow importing devDependencies

then under overrides:

  {
    files: [...configHelpers.devDependenciesFiles],
    rules: {
      'import/no-extraneous-dependencies': [
        'error',
        {
          packageDir: [process.cwd(), /* find the .git folder/file since this is published */],
        },
      ],
    },
  },

EDIT to add: I could do this in a separate PR if you prefer.

@ecraig12345
Copy link
Member

ecraig12345 commented Feb 12, 2021

eslint thing implemented in #16975

@Hotell Hotell force-pushed the hotell/16889/jest-svp branch from 2716789 to ba3f4c1 Compare February 16, 2021 11:23
@Hotell Hotell marked this pull request as ready for review February 16, 2021 11:24
ecraig12345
ecraig12345 approved these changes Feb 16, 2021
View reviewed changes
Copy link
Member

@ecraig12345 ecraig12345 left a comment

Choose a reason for hiding this comment

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

Thanks for doing this! Approved with one super minor suggestion. (In this case it's definitely fine to admin merge rather than chasing down all the package owners to approve.)

yarn.lock
resolved "https://registry.yarnpkg.com/axe-core/-/axe-core-3.5.0.tgz#161b17fc6ce07a5470ff179e0703957c422b2220"
integrity sha512-wJqOIreoFiGjvZ1UZvGLAUs8H3QQ3cS833+6ctFcCdr/xFd5oB66mmwGWnwQlBXFFaefRt+KR+m2dY9em2RxVg==

axe-core@^3.0.3:
Copy link
Member

@ecraig12345 ecraig12345 Feb 16, 2021

Choose a reason for hiding this comment

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

Probably not important in this case, but it looks like the addition of this extra axe-core version is unnecessary (maybe a leftover from some previous version of the change). So you could revert this diff and the one above.

Copy link
Contributor Author

@Hotell Hotell Feb 17, 2021

Choose a reason for hiding this comment

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

this was generated by yarn any manual change will make all contributors quite a hard time as after running yarn they would have modified changes.

Copy link
Member

@ecraig12345 ecraig12345 Feb 18, 2021

Choose a reason for hiding this comment

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

any manual change will make all contributors quite a hard time as after running yarn they would have modified changes.

It won't as long as:

  1. the combined version spec satisfies everything that's being requested (it would in this case--the full set of requested versions is the same before and after, so this actually shouldn't have changed at all; it probably only did as a leftover from some other intermediate changes)
  2. you run yarn yourself after any manual version tweaks, and include any further changes in the commit

This is something I've ended up doing a good bit of manually while we're still working on improving tooling in the area. It's more impactful when the initial duplicate is a package like @babel/core which has a whole big tree of dependencies.

ecraig12345
ecraig12345 approved these changes Feb 16, 2021
View reviewed changes
Copy link
Member

@ecraig12345 ecraig12345 left a comment

Choose a reason for hiding this comment

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

One minor suggestion but otherwise LGTM, thanks!

You may also need to check again in a week or so to ensure that no new packages (created before this PR or from outdated branches) added references to any of these deps. Or possibly add that as a check in lint-staged or elsewhere--not required now but an interesting thing to think about for the future, how to enforce the policy that certain deps are specified only at the root.

@Hotell Hotell merged commit 0de01d0 into microsoft:master Feb 17, 2021
@Hotell Hotell mentioned this pull request Feb 17, 2021
Merged
1 task
@layershifter layershifter mentioned this pull request Feb 19, 2021
Merged
2 tasks
joshualamusga1 pushed a commit to joshualamusga1/fluentui that referenced this pull request Feb 25, 2021
@Hotell @joshualamusga1
chore: apply single version policy on jest (microsoft#16950)
9632b83
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ecraig12345 ecraig12345 ecraig12345 approved these changes

@assuncaocharles assuncaocharles Awaiting requested review from assuncaocharles

@behowell behowell Awaiting requested review from behowell

@dzearing dzearing Awaiting requested review from dzearing dzearing is a code owner

@evlevy evlevy Awaiting requested review from evlevy

@joschect joschect Awaiting requested review from joschect

@jurokapsiar jurokapsiar Awaiting requested review from jurokapsiar

@khmakoto khmakoto Awaiting requested review from khmakoto khmakoto is a code owner

@kolaps33 kolaps33 Awaiting requested review from kolaps33

@layershifter layershifter Awaiting requested review from layershifter

@lorejoh12 lorejoh12 Awaiting requested review from lorejoh12

@miroslavstastny miroslavstastny Awaiting requested review from miroslavstastny

@pompomon pompomon Awaiting requested review from pompomon

@Raghurk Raghurk Awaiting requested review from Raghurk

@xugao xugao Awaiting requested review from xugao

+1 more reviewer

@ling1726 ling1726 ling1726 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@joschect joschect

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@Hotell @DustyTheBot @fabricteam @ecraig12345 @ling1726 @joschect