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

[WIP] Add createModernBuild option #188

Closed
wants to merge 1 commit into from
Closed

Conversation

davidtheclark
Copy link
Contributor

Closes #183. (Also, for kicks, closes #187.)

Inspired by https://philipwalton.com/articles/deploying-es2015-code-in-production-today/.

Adds an option createModernBuild, which defaults to true. When this mode is on, Batfish turns up the fanciness ...

I'm considering a "modern" build one that expects the browser to support ES2015 classes and arrow functions. I was thinking that no compilation of those two features might save us a lot, considering how often they are used and how bulky the class compilation is. However, there are some other ES2015 features we might want to expect, as well, for similar reasons:

  • for-of
  • template literals
  • parameters (e.g. default parameters)

Batfish hard-codes a babel-preset-env configuration that would create a build suited to browsers with the expected ES2015 features (see https://github.com/mapbox/batfish/compare/es2015-bundle?expand=1#diff-1fe5c18317c115506c68d98801aba837R17).

When you batfish start, it uses that config, assuming you're going to be doing your iterative development with a modern browser.

When you batfish build, Batfish builds two sets of client bundles, one for modern browsers and one for legacy browsers. (It only needs to build one static bundle still, but while investigating this I realized we can limit compilation on that static bundle to the features not available in your running version of Node.) So we end up with two copies of almost every file, the modern versions prefixed with m-. When we build the HTML, then, we include a runtime check for support of the ES2015 syntax we want (i.e. classes and arrow functions): if it's supported, load the modern bundles; if not, load the legacy bundles.

Another trick is that Uglify does not yet understand ES2015 syntax, so when minifying the modern bundles we need to use babel-minify, instead.

I've tried this out on several examples, using IE11 as the legacy browser — and it seemed to work nicely!

@jfurrow I'd love your review — affirmation that this makes, or criticism of it — before I update all the relevant tests.

@davidtheclark
Copy link
Contributor Author

It looks to me like if we wanted to remove compilation for template literals, default parameters, and for-of loops, all we'd need to do is bump the Edge version to 14.

Object.assign({}, batfishConfig, {
babelPresetEnvOptions: {
useBuiltIns: true,
targets: { node: 'current' }
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand why we only need to compile what's necessary for the current version of Node. Can you explain why this is the case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When we're using Node 6, we get a lot of ES2015 without compilation. So we should in theory have faster compile-time and maybe even faster runtime if, for example, we don't compile classes and arrow functions, which Node 6 understands.

Copy link
Contributor

Choose a reason for hiding this comment

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

So the set of ES features that get compiled, and end up in the static bundle, are driven by the version of Node that compiles the assets? If I'm understanding this correctly, I feel like this could result in some confusion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As we clarified in chat: The misunderstanding here was about where and when the static bundle runs. The output of this Webpack task is just static-render-pages.js, and that only runs in Node.

try {
eval('const a = x => 2; class Foo {}');
supportsModern = true;
} catch (e) {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason for the e argument (is it even an argument?)? Just curious!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only to please ESLint 🤷‍♂️

Copy link
Contributor

Choose a reason for hiding this comment

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

Ohhh I see!

@jfurrow
Copy link
Contributor

jfurrow commented Oct 3, 2017

This is super cool, and is working as expected. I say it's ready for the next step.

Nice work!

@@ -74,7 +70,7 @@ function createWebpackConfigBase(
presets: babelPresets,
plugins: babelPlugins,
babelrc: false,
compact: true
compact: false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Change this back to avoid those annoying Webpack messages about deoptimization.

Object.assign({}, batfishConfig, {
babelPresetEnvOptions: {
useBuiltIns: true,
targets: { node: 'current' }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As we clarified in chat: The misunderstanding here was about where and when the static bundle runs. The output of this Webpack task is just static-render-pages.js, and that only runs in Node.

@davidtheclark
Copy link
Contributor Author

Once Babel 7 is out I'm going to start clean on this idea.

@davidtheclark davidtheclark deleted the es2015-bundle branch March 23, 2018 20:11
@benjamn
Copy link

benjamn commented Jun 8, 2018

@davidtheclark In case you find it helpful, here's a blog post about how Meteor 1.7 implements modern/legacy bundling: https://blog.meteor.com/meteor-1-7-and-the-evergreen-dream-a8c1270b0901

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