-
Notifications
You must be signed in to change notification settings - Fork 832
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
chore: upgrade rollup #1068
chore: upgrade rollup #1068
Conversation
Signed-off-by: Yue Yang <[email protected]>
Signed-off-by: Yue Yang <[email protected]>
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
@mshwery Hi is there any progress on this PR? |
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.
Hey @g1eny0ung this looks good to me! Could you try running npx yarn-deduplicate && yarn install
? It looks like there were enough lockfile changes that there might be some overlap across deps.
@akleiner2 mind taking a look at this?
@mshwery Already done. 😆 |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Signed-off-by: Yue Yang <[email protected]>
✔️ Deploy Preview for evergreen-storybook ready! 🔨 Explore the source changes: 20fb1a0 🔍 Inspect the deploy log: https://app.netlify.com/sites/evergreen-storybook/deploys/60f7e70c0065e400083cf531 😎 Browse the preview: https://deploy-preview-1068--evergreen-storybook.netlify.app/ |
Signed-off-by: Yue Yang <[email protected]>
@mshwery @akleiner2 The stalebot's comment made me notice that there are some conflicts in this PR. I already fix them. PTAL again. |
Looks like this is failing on size-limit checks. I think we can bump slightly. |
Signed-off-by: Yue Yang <[email protected]>
@mshwery After resolving the conflict, I locally passed the size-limit check. Now CI also passed. |
I compared the output (unminified) and it's significantly different in size than the previous output. Seeing an initial output of 1.6mb → 1.8mb |
I would be ok merging this only because it's a tertiary output format (UMD) that isn't used by most folks, but would like more confidence in that build output size change. |
Signed-off-by: Yue Yang <[email protected]>
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.
Just tested again and you are correct! Not sure what was up with the last time I tested your PR.
Signed-off-by: Yue Yang [email protected]
Overview
This PR upgrade the rollup configuration, below are the changes:
namedExports
because the commonjs plugin can resolve named exports automatically nowrollup-plugin-babel
to@rollup/plugin-babel
. After my test, the bundling speed can be shortened by 2-3sScreenshots (if applicable)
Documentation