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

Core: De-duplicate babel use in core #28972

Merged
merged 19 commits into from
Aug 27, 2024
Merged

Core: De-duplicate babel use in core #28972

merged 19 commits into from
Aug 27, 2024

Conversation

ndelangen
Copy link
Member

@ndelangen ndelangen commented Aug 26, 2024

What I did

I investigated the bundle meta file from esbuild for the core, and I noticed this:
Screenshot 2024-08-26 at 12 27 43

Notice the exact same blocks 3 times in 3 entrypoints?
That's babel getting embedded 3 times.

Another way to look at it:
Screenshot 2024-08-26 at 12 28 26

And I wondered, if this duplication can be dealt with by making an entry-point that exposes all these babel/transform related types and functions.

And the results are pretty good:

before:
Screenshot 2024-08-26 at 12 11 45

after:
Screenshot 2024-08-27 at 12 09 13

before:
Screenshot 2024-08-26 at 12 27 43

after:
Screenshot 2024-08-27 at 12 09 18

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

Before this change prettier would always be used to format files when we storybook them.
(save from controls, modifying main.ts etc.)

Now that only happens if we can find prettier as a dep.
(it's an optional peerDependency of the core)

No further testing instruction here, because there should be no user impact.

Documentation

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli/src/sandbox-templates.ts

  • Make sure this PR contains one of the labels below:

    Available labels
    • bug: Internal changes that fixes incorrect behavior.
    • maintenance: User-facing maintenance tasks.
    • dependencies: Upgrading (sometimes downgrading) dependencies.
    • build: Internal-facing build tooling & test updates. Will not show up in release changelog.
    • cleanup: Minor cleanup style change. Will not show up in release changelog.
    • documentation: Documentation only changes. Will not show up in release changelog.
    • feature request: Introducing a new feature.
    • BREAKING CHANGE: Changes that break compatibility in some way with current major version.
    • other: Changes that don't fit in the above categories.

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>

name before after diff z %
createSize 0 B 0 B 0 B - -
generateSize 76.4 MB 76.4 MB 0 B 0.99 0%
initSize 169 MB 150 MB -18.9 MB -227.17 🔰-12.6%
diffSize 92.8 MB 73.9 MB -18.9 MB -1629.27 🔰-25.6%
buildSize 7.46 MB 7.46 MB 267 B 2.21 0%
buildSbAddonsSize 1.62 MB 1.62 MB 0 B 1.14 0%
buildSbCommonSize 195 kB 195 kB 0 B - 0%
buildSbManagerSize 2.3 MB 2.3 MB 0 B - 0%
buildSbPreviewSize 352 kB 352 kB 0 B 1.36 0%
buildStaticSize 0 B 0 B 0 B - -
buildPrebuildSize 4.46 MB 4.46 MB 0 B 1.23 0%
buildPreviewSize 3 MB 3 MB 267 B 2.78 0%
testBuildSize 0 B 0 B 0 B - -
testBuildSbAddonsSize 0 B 0 B 0 B - -
testBuildSbCommonSize 0 B 0 B 0 B - -
testBuildSbManagerSize 0 B 0 B 0 B - -
testBuildSbPreviewSize 0 B 0 B 0 B - -
testBuildStaticSize 0 B 0 B 0 B - -
testBuildPrebuildSize 0 B 0 B 0 B - -
testBuildPreviewSize 0 B 0 B 0 B - -
name before after diff z %
createTime 6.7s 21.1s 14.4s 0.58 68.2%
generateTime 19s 21.6s 2.6s 0.17 12%
initTime 15.7s 16s 304ms -1.04 1.9%
buildTime 11.9s 11.6s -290ms -1 -2.5%
testBuildTime 0ms 0ms 0ms - -
devPreviewResponsive 7.8s 7.1s -777ms -0.48 -10.9%
devManagerResponsive 4.7s 4.5s -124ms -0.69 -2.7%
devManagerHeaderVisible 735ms 844ms 109ms 0.43 12.9%
devManagerIndexVisible 767ms 885ms 118ms 0.43 13.3%
devStoryVisibleUncached 1.3s 1.1s -240ms -1.05 -21.5%
devStoryVisible 766ms 884ms 118ms 0.39 13.3%
devAutodocsVisible 732ms 655ms -77ms -0.96 -11.8%
devMDXVisible 630ms 714ms 84ms -0.08 11.8%
buildManagerHeaderVisible 678ms 681ms 3ms -0.96 0.4%
buildManagerIndexVisible 683ms 681ms -2ms -1.02 -0.3%
buildStoryVisible 717ms 758ms 41ms -0.62 5.4%
buildAutodocsVisible 690ms 617ms -73ms -1.45 🔰-11.8%
buildMDXVisible 631ms 614ms -17ms -1.04 -2.8%

Greptile Summary

This PR introduces changes to de-duplicate Babel usage in the Storybook core, aiming to reduce bundle size and improve performance.

  • Added new 'babel' export in @storybook/core package.json
  • Created centralized Babel entry point in 'code/core/src/babel/index.ts'
  • Updated imports across multiple files to use '@storybook/core/babel'
  • Added bundle analysis report generation in 'code/core/scripts/prep.ts'
  • Introduced new Babel-related exports in CLI package for internal use

@ndelangen ndelangen self-assigned this Aug 26, 2024
@ndelangen ndelangen added maintenance User-facing maintenance tasks ci:normal labels Aug 26, 2024
Copy link

nx-cloud bot commented Aug 26, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit c8decc2. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

greptile-apps[bot]

This comment was marked as resolved.

@ndelangen
Copy link
Member Author

How do you think this PR compares to yours here: #28571
@kasperpeulen ?

Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

Amazing work @ndelangen -- this is a truly stupendous improvement!!!!!! ❤️ ❤️ ❤️

Approving on conditions that:

  1. You've tested the metafile generation changes
  2. You've tested the prettier changes
  3. You document how to test in the PR description

Also, in the future it would be better to split the babel & prettier changes into two separate PRs. But I'm fine with the one PR since it's already prepared.

@ndelangen ndelangen merged commit 1de6e6f into next Aug 27, 2024
56 checks passed
@ndelangen ndelangen deleted the norbert/babel-dedup-core branch August 27, 2024 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:normal maintenance User-facing maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants