Skip to content

Expand Babel helpers#141

Merged
aomarks merged 4 commits intomasterfrom
full-babel-helpers
Apr 13, 2018
Merged

Expand Babel helpers#141
aomarks merged 4 commits intomasterfrom
full-babel-helpers

Conversation

@aomarks
Copy link
Member

@aomarks aomarks commented Apr 13, 2018

Background
ES5 compilation and ES to AMD module transformation rely on various Babel helper functions. In the case of Polyserve, we allow Babel to inject the helpers it needs into each script, as it encounters a need for them (meaning they are potentially duplicated many times across an app). For build, though, we instead inject a static set of helpers into the entrypoint HTML document, which is generated by our gulpfile. This set of helpers has become out of date.

This PR

  • Adds a bunch of missing Babel helpers. Includes ES2017 and ES2018 plugin-related helpers, and even some missing ES2015 ones. I did some research to figure out which plugins depend on which helpers, and marked up the list with comments.

  • Adds the two module transform helpers interopRequireDefault and interopRequireWildcard to our AMD loader script. This way builds that do the AMD transform but do not compile to ES5 will not depend on the main Babel helpers, and builds that compile to ES5 but don't do the AMD transform will not depend on the Babel helpers.

For the longer term, I filed #128 to track switching to using the transform-runtime plugin, which would mean Babel helper dependencies are expressed as ES module imports, which should simplify this whole process greatly, and unify the logic between polyserve and build.

I manually tested that this fixes the shop#3.0 es6+amd build, and fixes at least the Babel helper-related errors from the es5 build.

Fixes https://github.com/Polymer/polymer-build/issues/358

@aomarks aomarks requested a review from rictic April 13, 2018 22:39
Copy link
Contributor

@rictic rictic left a comment

Choose a reason for hiding this comment

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

Great work!

These tests though… they're really opaque. What can we do to better validate that the output is doing what we want?

@rictic
Copy link
Contributor

rictic commented Apr 13, 2018

Moving to importing babel helpers will be really really helpful for legible output, but I guess that's not ready yet?

@aomarks
Copy link
Member Author

aomarks commented Apr 13, 2018

These tests though… they're really opaque. What can we do to better validate that the output is doing what we want?

Agreed. The only thing you can really gather from the diff shown here is that the require.js script has gotten slightly longer, and the main babel helper script has gotten quite a bit longer. I've already filed https://github.com/Polymer/polymer-cli/issues/990 to improve these goldens. If we replace the known script chunks with comments, we could have goldens like:

<script>
// amd loader
// babel module helpers
</script>

<script>
// babel main helpers
</script>

Also the e2e integration tests you're working on would give us confidence that these features are working without being tied to any golden output.

Moving to importing babel helpers will be really really helpful for legible output, but I guess that's not ready yet?

Yeah! I'm not sure how ready it is on the Babel side, will need investigate. Don't want to change too may things at once right now, but will go back to this once some of the other pressing tooling bugs get fixed. One issue is that it will force existing non-es-module projects that want to compile to ES5 to turn on the AMD transform. Another is that we have to think through how this interacts with bundler: currently bundling runs before the Babel transform, which would mean bundler has no chance to bundle the helpers. That might mean splitting up build to do ES5 compilation first, then bundling, then AMD transform.

@aomarks aomarks merged commit 463353d into master Apr 13, 2018
@aomarks aomarks deleted the full-babel-helpers branch April 13, 2018 23:36
aomarks added a commit that referenced this pull request Apr 13, 2018
aomarks added a commit that referenced this pull request Apr 13, 2018
@shawncplus
Copy link

@aomarks I originally encountered the babel helpers issue when trying to use async/await in a 1.x project build with polymer-build. Will this work backwards to 1.x or is it only intended for 3.0?

@aomarks
Copy link
Member Author

aomarks commented Apr 14, 2018

@aomarks I originally encountered the babel helpers issue when trying to use async/await in a 1.x project build with polymer-build. Will this work backwards to 1.x or is it only intended for 3.0?

This will work with older HTML import style projects too, not just module style ones.

aomarks added a commit that referenced this pull request Apr 17, 2018
In #141 I made a change whereby the two Babel helpers needed for the AMD transform were added to our require.js AMD loader script (since they go hand-in-hand). However, that meant in the (rather common) case that you both ES5 compiled and AMD transformed, our require.js script would clobber the main AMD helpers script. This is because the helper code generated by AMD always overwrites window.babelHelpers, rather than augmenting it.

In this PR I instead introduce two different Babel helper scripts: "full" and "amd". "full" includes the AMD helpers along with everything else, while "amd" only contains the AMD helpers.

This is all kind of gross and I want to vastly simplify it via #128, but this solution should unblock us in the short term.

Fixes #147
@ghost ghost mentioned this pull request Jan 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants