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

Wrap babel-runtime@^6 dependency #660

Closed
novemberborn opened this issue Mar 20, 2016 · 7 comments
Closed

Wrap babel-runtime@^6 dependency #660

novemberborn opened this issue Mar 20, 2016 · 7 comments

Comments

@novemberborn
Copy link
Member

Many Babel dependencies depend on babel-runtime@^5. AVA depends on babel-runtime@^6. Often this leads to npm not deduping the babel-runtime@^5 dependency, causing install size to balloon.
#400 rewrites babel-runtime require statements in the transpiled test files to absolute paths. This would allow us to wrap babel-runtime@^6 in an intermediate package, say ava-babel-runtime. npm may then be more likely to dedupe babel-runtime@^5.

Of course this would only apply to AVA itself. Other dependencies may be introduced by consuming packages which cause babel-runtime@^6 to not be deduped. Whether this will impact real world install size is to be determined. See #369.

@jamestalmage
Copy link
Contributor

I would just as soon see this fixed in babel. I have commented on an already open issue here.

As you mention, this doesn't help if users need babel-runtime@6 anyways (which seems increasingly likely).

Also, it's not quite so trivial as wrapping babel-runtime@5. Your wrapper can't just do this:

// unfortunately, our wrapper won't be this simple
module.exports = require('babel-runtime');

babel-plugin-transform-runtime actually creates require statements with some pretty deep paths.

Here are the first few transpiled lines of babel-plugin-transform-es2015-blocking-scope:

var _classCallCheck = require("babel-runtime/helpers/class-call-check")["default"];

var _Object$create = require("babel-runtime/core-js/object/create")["default"];

var _Symbol = require("babel-runtime/core-js/symbol")["default"];

var _interopRequireDefault = require("babel-runtime/helpers/interop-require-default")["default"];

var _interopRequireWildcard = require("babel-runtime/helpers/interop-require-wildcard")["default"];

We definitely could do this ourselves (and if the babel team decides they won't, that will be our only option). I just think fixing it in babel would be easier and better for the community at large.

@sindresorhus
Copy link
Member

I just think fixing it in babel would be easier and better for the community at large.

Agreed. Let's keep this open so we don't forget to follow-up with Babel. We can discuss doing this if they don't fix it in the short-term.

@novemberborn
Copy link
Member Author

@jamestalmage ah hadn't looked into it that deeply. Yikes.

@sindresorhus
Copy link
Member

Looks like babel/babel#3438 will solve our babel-runtime problem.

@hzoo
Copy link

hzoo commented May 3, 2016

Merged babel/babel#3438 and released in 6.8.0

@jamestalmage
Copy link
Contributor

Awesome. I just tried it out on a random project, and it "feels" faster (could be placebo effect though).

Have you done any profiling with the change?

@hzoo
Copy link

hzoo commented May 3, 2016

No, I haven't - we had some discussions on how we could reliably do that but I'm not sure if we came up with anything since running a benchmark on travis didn't seem to be a good idea given the variance. I think it "felt" faster to me too lol

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants