Moving babel-polyfill into devDependencies as it's adding bloat

[joelgriffith]

TL;DR This leaks through to downstream builds/packages, so moving it out.

In my use-case: jest sees that there's a babel-polyfill package in node_modules, and uses it. In a CI environment this hits a particular memory issue wherein our box runs out of available heap for tests to run, and tests die.

I think babel-polyfill's inclusion into dependencies may be accidental, however it should really only be included in dev and not downstream packages.

[stubailo]

however it should really only be included in dev and not downstream packages.

It's a runtime dependency though right? Some code generated by babel might not work without it. I guess we could turn on babel-transform-runtime though to get rid of it?

[joelgriffith]

Perhaps... TBH I'm not intimately familiar with how eslint plugins are shipped, my naive assumption is that they're compiled, so no need for any babel packages in dependencies (which was what I gathered having looked at the package.json in this project).

Does this plugin compile/run on the fly?

[stubailo]

Babel has two parts:

1. All of the plugins, which run at build time
2. Polyfills/runtime, which need to be included for the compiled code to run

babel-polyfill is in the second category - it's needed for the code to run.

According to the docs babel-polyfill shouldn't be used in a tool or library, so you're right that it should be changed. The docs suggest using babel-transform-runtime instead:

If you are looking for something that won't modify globals to be used in a tool/library, checkout the transform-runtime plugin. This means you won't be able to use the instance methods mentioned above like Array.prototype.includes.

So we should be using babel-transform-runtime instead: http://babeljs.io/docs/plugins/transform-runtime/

I'd be happy to merge a PR that makes this switch. That means we would have babel-runtime instead of babel-polyfill as a runtime dependency.

[mute]

Another thing to consider is that for it to be effective at runtime it would need to be required in the transpiled build of the plugin here, which it isn't right now:
https://unpkg.com/[email protected]

The only place that I can find that it's used in the code for eslint-plugin-graphql is in the startup script for the tests here: https://github.com/apollographql/eslint-plugin-graphql/blob/master/test/index.js

It makes sense that it would be required in the tests because you might want to control for your CI environment running on an older version of node which might need babel-polyfill, but, as far as I can tell, it's not actually being used in the published package for this library right now.

[stubailo]

OK I'm convinced! Sorry for the confusion, I haven't looked into the build setup for this package in a while.

[joelgriffith]

Thanks @stubailo!

[stubailo]

Published in 0.6.1, heads up @jnwng