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

Remove Babel helper binding renaming #34

Merged
merged 1 commit into from
Jan 16, 2016

Conversation

ericf
Copy link
Contributor

@ericf ericf commented Jan 13, 2016

The attempt to fix the Babel helper binding renaming in #31 didn't work for all cases. The transform-object-rest-spread uses:

babelHelpers['extends']

It seems safer to just remove this optimization, and let the helpers stay named babelHelpers..

The attempt to fix the Babel helper binding renaming in rollup#31 didn't
work for all cases. The `transform-object-rest-spread` uses:

```js
babelHelpers['extends']
```

It seems safer to just remove this optimization, and let the
helpers stay named `babelHelpers.`.
@eventualbuddha
Copy link
Contributor

Could we just make Babel use a more consistent format?

@ericf
Copy link
Contributor Author

ericf commented Jan 13, 2016

@eventualbuddha isn't it up the plugin author to determine how the helper is invoked? Or add extra helpers to the collection for their plugin?

@Victorystick
Copy link
Contributor

One possible way to get the best of both worlds would be to import a virtual, ES2015 version of the babel-helpers where one or more helper are used.

import * as babelHelpers from 'babel-helpers';

// This access will be optimised automatically by Rollup.
babelHelpers.createClass(...);

// Rollup will make sure this dynamic access will work
// while preserving all other optimised accesses.
babelHelpers[ dynamic ](...);

When babel-helpers is imported, we inject a virtual module like so:

return buildExternalHelpers( all, 'var' )
  .replace( /var babelHelpers = {};\n/, '' ) // strip declaration
  .replace( /babelHelpers\.(.+) = /g, 'export var $1 = ' ) // export the helper
  .replace( 'babelHelpers;', '' ) // not sure where this comes from...
  .trim() + '\n';

Thoughts?

@ericf
Copy link
Contributor Author

ericf commented Jan 14, 2016

Would you guys be up for merging this to fix the immediate issue of Rollup producing code that errors when certain Babel plugins are being used? And in parallel we can get a more robust solution in place and explore ideas like @Victorystick's?

@eventualbuddha
Copy link
Contributor

I'm fine with that.
On Thu, Jan 14, 2016 at 8:37 AM Eric Ferraiuolo notifications@github.com
wrote:

Would you guys be up for merging this to fix the immediate issue of Rollup
producing code that errors when certain Babel plugins are being used? And
in parallel we can get a more robust solution in place and explore ideas
like @Victorystick https://github.com/Victorystick's?


Reply to this email directly or view it on GitHub
#34 (comment)
.

@Victorystick
Copy link
Contributor

@ericf Absolutely. 👍

I've just realised that we'll run into trouble with babelHelpers.typeof = ... using my approach. We cannot transform it into an exported var declaration. It'll result in a SyntaxError. If only Babel had chosen to name it typeOf instead... 😭

export var typeof = ...

export var typeOf = ...

@ericf
Copy link
Contributor Author

ericf commented Jan 15, 2016

@Victorystick @eventualbuddha okay great! Let me know if there's anything else I can do to help you guys get a patch release out.

@eventualbuddha eventualbuddha merged commit dec1704 into rollup:master Jan 16, 2016
@eventualbuddha
Copy link
Contributor

Released in v2.3.9.

@ericf
Copy link
Contributor Author

ericf commented Jan 17, 2016

Thanks guys!

@ericf ericf deleted the remove-babel-helper-renaming branch January 17, 2016 04:51
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.

3 participants