-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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) create-package script now works when creating a converged node package #23196
(fix) create-package script now works when creating a converged node package #23196
Conversation
@@ -17,7 +17,7 @@ const v8ReferencePackages = { | |||
}; | |||
const convergedReferencePackages = { | |||
react: ['react-provider'], | |||
node: ['babel-make-styles'], | |||
node: ['react-components/react-utilities'], |
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.
If I should be referencing some other package in this case, please let me know!
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 should not be referencing this package, since it is a react package.
The only 'real' node package that we have left since Griffel moved out would be bundle-size
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.
IMO it should be empty array.
I'm working on proper fix (still need to align with what to put in the array). will link here the PR once it will be ready
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.
So after looking into it, when I run the script using react-utilities
as the reference in comparison to bundle-size
- everything generated is exactly the same besides the package.json
. The only difference is that bundle-size
does not have a dependency on tslib
, @fluentui/eslint-plugin
and @fluentui/scripts
and it also doesn't have a beachball config so it actually ends up erroring out. Now I could try to fix this by modifying the script but it doesn't seem to make sense to do so when I could get the output i'm looking for that react-utilities
would already give me.
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.
#23249 - this fixes creation of react v9 and v8 stuff.
IMO the logic should be the other way around, something like following:
- it checks packages defined in new package template
- fetches latest monorepo versions of those packages
- updates new package.json with those version
This issue demonstrates "the hard requirement" for a need to provide a reference package - which doesn't make sense in this case as I mentioned previously.
WDYT folks ?
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.
Oops sorry, should have refreshed this page first since I didn't see your previous comment before I commented 😅. But sounds good to me! I'll close this PR in favor of yours :)
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 worries dude, I saw it also after my comment, so I guess your arrived first :D
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 1e40766:
|
Asset size changesSize Auditor did not detect a change in bundle size for any component! Baseline commit: 3321df44cdc17b93c110019f7365bc80f0004e1b (build) |
Perf Analysis (
|
Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
---|---|---|---|---|---|
Avatar | mount | 912 | 917 | 5000 | |
Button | mount | 581 | 561 | 5000 | |
FluentProvider | mount | 1887 | 1874 | 5000 | |
FluentProviderWithTheme | mount | 294 | 268 | 10 | |
FluentProviderWithTheme | virtual-rerender | 241 | 258 | 10 | |
FluentProviderWithTheme | virtual-rerender-with-unmount | 321 | 306 | 10 | |
MakeStyles | mount | 1568 | 1573 | 50000 |
Perf Analysis (
|
Scenario | Current PR Ticks | Baseline Ticks | Ratio |
---|---|---|---|
PortalMinimalPerf.default | 150 | 138 | 1.09:1 |
AvatarMinimalPerf.default | 159 | 149 | 1.07:1 |
AttachmentMinimalPerf.default | 132 | 125 | 1.06:1 |
AlertMinimalPerf.default | 234 | 223 | 1.05:1 |
TextMinimalPerf.default | 287 | 274 | 1.05:1 |
ImageMinimalPerf.default | 313 | 300 | 1.04:1 |
LabelMinimalPerf.default | 317 | 305 | 1.04:1 |
SkeletonMinimalPerf.default | 293 | 282 | 1.04:1 |
TreeWith60ListItems.default | 136 | 131 | 1.04:1 |
AttachmentSlotsPerf.default | 938 | 914 | 1.03:1 |
CarouselMinimalPerf.default | 397 | 386 | 1.03:1 |
ChatMinimalPerf.default | 626 | 610 | 1.03:1 |
ButtonSlotsPerf.default | 472 | 463 | 1.02:1 |
CardMinimalPerf.default | 467 | 460 | 1.02:1 |
LayoutMinimalPerf.default | 298 | 291 | 1.02:1 |
ListCommonPerf.default | 525 | 517 | 1.02:1 |
ListMinimalPerf.default | 426 | 416 | 1.02:1 |
ListWith60ListItems.default | 514 | 503 | 1.02:1 |
LoaderMinimalPerf.default | 568 | 558 | 1.02:1 |
RosterPerf.default | 953 | 936 | 1.02:1 |
PopupMinimalPerf.default | 534 | 522 | 1.02:1 |
RadioGroupMinimalPerf.default | 373 | 366 | 1.02:1 |
ToolbarMinimalPerf.default | 807 | 790 | 1.02:1 |
BoxMinimalPerf.default | 285 | 281 | 1.01:1 |
ButtonOverridesMissPerf.default | 1261 | 1246 | 1.01:1 |
DialogMinimalPerf.default | 640 | 636 | 1.01:1 |
FormMinimalPerf.default | 330 | 327 | 1.01:1 |
HeaderSlotsPerf.default | 626 | 621 | 1.01:1 |
MenuMinimalPerf.default | 709 | 701 | 1.01:1 |
MenuButtonMinimalPerf.default | 1437 | 1420 | 1.01:1 |
ProviderMergeThemesPerf.default | 1076 | 1061 | 1.01:1 |
ProviderMinimalPerf.default | 339 | 336 | 1.01:1 |
SliderMinimalPerf.default | 1439 | 1420 | 1.01:1 |
SplitButtonMinimalPerf.default | 3708 | 3664 | 1.01:1 |
StatusMinimalPerf.default | 562 | 557 | 1.01:1 |
TableManyItemsPerf.default | 1611 | 1595 | 1.01:1 |
TreeMinimalPerf.default | 676 | 672 | 1.01:1 |
VideoMinimalPerf.default | 540 | 536 | 1.01:1 |
AnimationMinimalPerf.default | 461 | 459 | 1:1 |
DividerMinimalPerf.default | 292 | 292 | 1:1 |
DropdownMinimalPerf.default | 2566 | 2560 | 1:1 |
EmbedMinimalPerf.default | 3489 | 3492 | 1:1 |
GridMinimalPerf.default | 283 | 282 | 1:1 |
HeaderMinimalPerf.default | 292 | 293 | 1:1 |
InputMinimalPerf.default | 1094 | 1093 | 1:1 |
ItemLayoutMinimalPerf.default | 991 | 989 | 1:1 |
ReactionMinimalPerf.default | 306 | 307 | 1:1 |
TextAreaMinimalPerf.default | 408 | 407 | 1:1 |
CheckboxMinimalPerf.default | 2265 | 2278 | 0.99:1 |
DropdownManyItemsPerf.default | 547 | 553 | 0.99:1 |
RefMinimalPerf.default | 190 | 191 | 0.99:1 |
CustomToolbarPrototype.default | 2320 | 2333 | 0.99:1 |
TooltipMinimalPerf.default | 950 | 959 | 0.99:1 |
AccordionMinimalPerf.default | 121 | 123 | 0.98:1 |
ButtonMinimalPerf.default | 130 | 133 | 0.98:1 |
ChatDuplicateMessagesPerf.default | 235 | 239 | 0.98:1 |
FlexMinimalPerf.default | 226 | 230 | 0.98:1 |
IconMinimalPerf.default | 496 | 507 | 0.98:1 |
TableMinimalPerf.default | 327 | 333 | 0.98:1 |
DatepickerMinimalPerf.default | 4731 | 4893 | 0.97:1 |
ListNestedPerf.default | 457 | 470 | 0.97:1 |
ChatWithPopoverPerf.default | 309 | 322 | 0.96:1 |
SegmentMinimalPerf.default | 274 | 285 | 0.96:1 |
📊 Bundle size reportUnchanged fixtures
|
Perf Analysis (
|
Scenario | Render type | Master Ticks | PR Ticks | Iterations | Status |
---|---|---|---|---|---|
BaseButton | mount | 787 | 766 | 5000 | |
Breadcrumb | mount | 2404 | 2346 | 1000 | |
Checkbox | mount | 1275 | 1292 | 5000 | |
CheckboxBase | mount | 1069 | 1105 | 5000 | |
ChoiceGroup | mount | 4090 | 4077 | 5000 | |
ComboBox | mount | 846 | 859 | 1000 | |
CommandBar | mount | 9044 | 9173 | 1000 | |
ContextualMenu | mount | 10084 | 10217 | 1000 | |
DefaultButton | mount | 974 | 990 | 5000 | |
DetailsRow | mount | 3359 | 3296 | 5000 | |
DetailsRowFast | mount | 3341 | 3361 | 5000 | |
DetailsRowNoStyles | mount | 3236 | 3197 | 5000 | |
Dialog | mount | 1967 | 1976 | 1000 | |
DocumentCardTitle | mount | 155 | 160 | 1000 | |
Dropdown | mount | 2861 | 2848 | 5000 | |
FocusTrapZone | mount | 1575 | 1621 | 5000 | |
FocusZone | mount | 1571 | 1593 | 5000 | |
IconButton | mount | 1518 | 1518 | 5000 | |
Label | mount | 297 | 298 | 5000 | |
Layer | mount | 2550 | 2571 | 5000 | |
Link | mount | 395 | 404 | 5000 | |
MenuButton | mount | 1286 | 1278 | 5000 | |
MessageBar | mount | 1861 | 1899 | 5000 | |
Nav | mount | 2845 | 2832 | 1000 | |
OverflowSet | mount | 931 | 953 | 5000 | |
Panel | mount | 1924 | 1895 | 1000 | |
Persona | mount | 864 | 864 | 1000 | |
Pivot | mount | 1257 | 1263 | 1000 | |
PrimaryButton | mount | 1133 | 1140 | 5000 | |
Rating | mount | 6720 | 6697 | 5000 | |
SearchBox | mount | 1107 | 1158 | 5000 | |
Shimmer | mount | 2178 | 2170 | 5000 | |
Slider | mount | 1701 | 1682 | 5000 | |
SpinButton | mount | 4224 | 4405 | 5000 | |
Spinner | mount | 375 | 381 | 5000 | |
SplitButton | mount | 2982 | 2775 | 5000 | |
Stack | mount | 457 | 447 | 5000 | |
StackWithIntrinsicChildren | mount | 2000 | 1980 | 5000 | |
StackWithTextChildren | mount | 4564 | 4507 | 5000 | |
SwatchColorPicker | mount | 10120 | 10166 | 5000 | |
TagPicker | mount | 2334 | 2393 | 5000 | |
TeachingBubble | mount | 89272 | 88418 | 5000 | |
Text | mount | 378 | 375 | 5000 | |
TextField | mount | 1230 | 1235 | 5000 | |
ThemeProvider | mount | 1034 | 1046 | 5000 | |
ThemeProvider | virtual-rerender | 556 | 565 | 5000 | |
ThemeProvider | virtual-rerender-with-unmount | 1638 | 1644 | 5000 | |
Toggle | mount | 697 | 685 | 5000 | |
buttonNative | mount | 112 | 109 | 5000 |
Issue now handled by #23249 |
Current Behavior
converged
node
package with theyarn create-package
script, the console errors out since the referenced package (babel-make-styles
) has long been removed from the monorepo by chore: remove babel-make-styles package #21482.New Behavior
converged
node
package is nowreact-utilities
.Related Issue(s)
Partially Fixes #22960