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

WIP: Bundle @babel/standalone with rollup #7581

Closed
wants to merge 10 commits into from
Closed

WIP: Bundle @babel/standalone with rollup #7581

wants to merge 10 commits into from

Conversation

danez
Copy link
Member

@danez danez commented Mar 15, 2018

Q                       A
Fixed Issues?
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR
Any Dependency Changes? rollup stuff
License MIT

This is still very early and super messy right now. Also it does not yet work completely as long as rollup/rollup#2059 isn't released. Locally I got the tests already passing.

So far it looks like the bundle size will decrease (I might have some more ideas for example excluding debug in the bundle):
babel.js 2.7MB -> 1.5MB
babel.min.js 1.4MB -> 0.8MB

Depends on :

Gulpfile.js Outdated
}
},
resolveId(importee) {
const matches = importee.match(/^@babel\/([^/]+)$/);
Copy link
Member

Choose a reason for hiding this comment

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

I've created https://github.com/Andarist/lerna-alias pretty much for this exact thing - aliasing to respective src.

Copy link
Member Author

@danez danez Mar 16, 2018

Choose a reason for hiding this comment

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

But it does not replace lib with src in the main entry?

Copy link
Member

Choose a reason for hiding this comment

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

It does exactly this (or I've misunderstood your question) - it searches for packages managed by lerna (using lerna's algo - although it's copied) and creates an alias map from PKG_NAME to PKG_PATH/src

Gulpfile.js Outdated
"package.json"
));

const filename =
Copy link
Member

@Andarist Andarist Mar 15, 2018

Choose a reason for hiding this comment

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

does any of our packages use browser field that should be resolved here? rollup-plugin-node-resolve handles browser field if u pass browser: true as an option to it - which I see that you have used

Copy link
Member Author

@danez danez Mar 15, 2018

Choose a reason for hiding this comment

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

The problem is that the browser field is using /lib/ and not /src/ so the node plugin won't pick it up.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, ok - I've searched for a simple "browser" fields with string value, I see we have some object maps there in i.e. @babel/core.

I think it might be possible to workaround this easier - rename our browser files to have .browser.js extension and make rollup-plugin-node-resolve to pick it up instead of the original one by passing extensions: ['.browser.js', '.js'] option

Copy link
Member

Choose a reason for hiding this comment

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

I'd assume it is fine for the browser field map both src and lib for Babel at least. Would that work? e.g.

  "browser": {
    "./lib/config/files/index.js": "./lib/config/files/index-browser.js",
    "./lib/transform-file.js": "./lib/transform-file-browser.js",
    "./lib/transform-file-sync.js": "./lib/transform-file-sync-browser.js"
    "./src/config/files/index.js": "./src/config/files/index-browser.js",
    "./src/transform-file.js": "./src/transform-file-browser.js",
    "./src/transform-file-sync.js": "./src/transform-file-sync-browser.js"
  },

Copy link
Member

Choose a reason for hiding this comment

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

yeah, this definitely should work too 👍

@babel-bot
Copy link
Collaborator

babel-bot commented Mar 19, 2018

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/9393/

@Andarist
Copy link
Member

What's the status of this PR? Is this still something we'd like to pursue? I could work on rebasing this and taking this through the finish line.

@danez
Copy link
Member Author

danez commented Nov 16, 2018

It is finally working.
The savings for babel-standalone is 497kB vs 1.0 MB
babel-preset-env-standalone is 302kB vs 320kB

I wonder why babel-standalone is 50% smaller. In the repl everything works correctly, besides typescript (but typescript is also not working without this PR for me.).

I will now cleanup the PR and do some more stats.

@Andarist
Copy link
Member

Do we intend to support IE11 in the repl? If yes then would be great to check if its working there.

I wonder why babel-standalone is 50% smaller.

It depends on the previous setup, I've seen such reductions before when migrating from webpack to rollup (well, maybe not that big ones, but I've seen 30-40% reductions). If you are curious about it then the easiest way to find out would be to compare what was in the final bundle before and what is now - source map explorer could come in handy for that.

@JLHwung
Copy link
Contributor

JLHwung commented Nov 29, 2019

Closing it in favour of #10779.

@danez Thanks for your previous work on Rollup bundling, from which I learned and benefited a lot when working on #10779.

@JLHwung JLHwung closed this Nov 29, 2019
@JLHwung JLHwung deleted the rollup-bundle branch November 29, 2019 19:42
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Feb 29, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: perf outdated A closed issue/PR that is archived due to age. Recommended to make a new issue pkg: standalone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants