Skip to content
This repository has been archived by the owner on Aug 4, 2021. It is now read-only.

Adding support for referencing external, global helpers #41

Merged
merged 1 commit into from
Feb 23, 2016

Conversation

joeldenning
Copy link
Contributor

This adds a backwards-compatible option for indicating that you want to call babel helpers on the global babelHelpers object, instead of inlining the functions for this bundle.

I hesitated to add an option to the plugin that is not part of the normal babel options, but saw no other way to add in this feature without breaking backwards compatibility of code already using rollup-plugin-babel. The reason why it could not be solved with any combination of plugins is because rollup-plugin-babel assumes that babelHelpers need to be bundled even when you use the external-helpers or external-helpers-2 plugins. Also see here and here for the relevant code I'm referring to.

If there is some combination of external-helpers plugins that works without this pull request, then I'm all ears and will gladly switch to that and close this :)

@TrySound
Copy link
Member

@joeldenning Thanks. Squash commits please.

@joeldenning
Copy link
Contributor Author

Just squashed the commits. When would you plan on publishing to npm?

@TrySound
Copy link
Member

/cc @Rich-Harris @eventualbuddha

@@ -103,6 +107,7 @@ export default function babel ( options ) {
intro () {
const helpers = Object.keys( bundledHelpers );
if ( !helpers.length ) return '';
if ( externalHelpers ) return '';
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be moved to the top of the method, as we can return immediately if we're using external helpers. I'm liking the changes overall! 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point -- just changed that.

@Rich-Harris
Copy link
Contributor

Thanks for the PR @joeldenning. There is an existing option called runtimeHelpers which I think does what you need, though I could be mistaken? https://github.com/rollup/rollup-plugin-babel#helpers I'm not clear on where babelHelpers would come from otherwise?

Incidentally externalHelpers are so-called because they're external to the file Babel is currently dealing with, but it's a bit of a misnomer in our context because the helpers remain internal to the bundle – all of which probably adds to the confusion :-/

@joeldenning
Copy link
Contributor Author

@Rich-Harris http://www.2ality.com/2015/12/babel6-helpersstandard-library.html is a really good article that explains the difference between babel-polyfill, babel-runtime, and external-helpers-2. Especially take a look at 2.2, which explains the external-helpers-2 plugin.

In a nutshell, babel-runtime will put something like var runtime = require('babel-runtime') inside of your file and then reference the runtime variable throughout your module. On the other hand, external-helpers-2 assumes that there is a global object called babelHelpers and so it will not require anything at all, but rather just reference the global variable. Additionally, there are some nuanced differences in that article about how babel-runtime does polyfilling a bit differently than if you use babel-polyfill.

The case for using external-helpers-2 is (at least) twofold:
(1) Those nuanced differences I just mentioned, related to polyfilling es5, es6, and es7
(2) If you have more than one rollup bundle on the page, then it is better to not require('babel-runtime') with each of them but rather have the helpers external to all of them. Especially if you're using rollup-plugin-node-resolve and rollup-plugin-commonjs along with rollup-plugin-babel, since that would mean duplicate code in each of the bundles. And finally, if you're interested in why anybody would want to have more than one rollup bundle on a web page, I'd be happy to dive into that (one of the reasons is for interoperability of code across multiple SPAs).

Anyway, all that explanation was just to arrive at the conclusion that, without my pull request, it is impossible for the external-helpers-2 plugin to do what it is supposed to do (namely to reference a global variable called babelHelpers). As I explained in the original description of this pr, I considered doing a bigger refactor of rollup-plugin-babel so that it did not assume that you always want to bundle in the babelHelpers. But I decided against it since that would break backwards compatibility of code that is depending on rollup-plugin-babel refusing to reference the global babelHelpers.

@joeldenning
Copy link
Contributor Author

Ping

@Victorystick
Copy link
Contributor

@joeldenning It looks good to me.

Victorystick added a commit that referenced this pull request Feb 23, 2016
Adding support for referencing external, global helpers
@Victorystick Victorystick merged commit e02dcdb into rollup:master Feb 23, 2016
@Victorystick
Copy link
Contributor

Released in v2.4.0.

@joeldenning joeldenning deleted the external-helpers branch February 23, 2016 16:50
@joeldenning
Copy link
Contributor Author

Great thanks!

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

Successfully merging this pull request may close these issues.

4 participants