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

Adding Babel to frontend workflow; Upgrading Webpack to version 2. #2736

Merged
merged 3 commits into from
Mar 2, 2017

Conversation

sebworks
Copy link
Contributor

@sebworks sebworks commented Feb 8, 2017

Adding babel to frontend workflow; Upgrading Webpack to version 2.

Additions

  • Added .babelrc as a babel Configuration file. Using the es2015 and stage-2 presets. Presets are just sets of transformations.

Changes

  • Modified config/webpack-config.js to do the following:

    • Use the Babel loader
    • Enable the cache property for performance purposes.
    • Delete the second chunk entry which seems no longer necessary in Webpack 2.
  • Modified gulp/tasks/scripts.js to pass webpack-stream a Webpack param.

  • Updated package.json to do the following:

    • Add the necessary modules to support Babel.
    • Remove slick-carousel as it's no longer needed.
    • Move snyk as it's not necessary to run on Jenkins.

Testing

  • Run rm -rf node_modules
  • Run frontend.sh.
  • Browse the site as expected.
  • Run rm -rf node_modules
  • Run frontend.sh production.
  • Browse the site as expected.

Screenshots

Before:

screen shot 2017-02-08 at 12 26 39 pm

After:

screen shot 2017-02-08 at 11 48 54 am

Notes

Todos

  • Converting some modules over to ES6.

Checklist

  • PR has an informative and human-readable title
  • Changes are limited to a single goal (no scope creep)
  • Code can be automatically merged (no conflicts)
  • Code follows the standards laid out in the front end playbook
  • Passes all existing automated tests
  • New functions include new tests
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged
  • Visually tested in supported browsers and devices
  • Project documentation has been updated
  • Reviewers requested with the Assignee tool ➡️

@@ -0,0 +1,9 @@
{
"presets": ["es2015", "stage-2"],
Copy link
Member

Choose a reason for hiding this comment

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

Just curious: Why'd you choose stage 2? Have you looked at babel-preset-env that lets you target the browsers your project supports? (Although now that I think about it, because we support old versions of IE it probably would end up applying every plugin and transform imaginable).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why'd you choose stage 2?

It was just and arbitrary starting point. We can adjust if we find language features we want to use but find missing. The process is quite interesting, as it appears features are moving in / out of stages based on the TC39 process. https://github.com/tc39/proposals.

Copy link
Member

Choose a reason for hiding this comment

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

Works for me. 👍

@ascott1
Copy link
Member

ascott1 commented Feb 13, 2017

@sebworks along with this PR, we should add ES6 guidelines to our JS standards

package.json Outdated
@@ -69,6 +71,7 @@
"psi": "2.0.4",
"selenium-webdriver": "2.53.3",
"sinon": "1.17.3",
"snyk": "1.25",
Copy link
Member

Choose a reason for hiding this comment

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

1.25 -> 1.25.0

@sebworks
Copy link
Contributor Author

sebworks commented Feb 14, 2017

@sebworks along with this PR, we should add ES6 guidelines to our JS standards.

@ascott1 ok, I will work on the guidelines.

@anselmbradford
Copy link
Member

Will try to review this today!

filename: '[name]'
},
plugins: [
new webpack.optimize.CommonsChunkPlugin( COMMON_BUNDLE_NAME ),
new webpack.optimize.CommonsChunkPlugin( COMMON_BUNDLE_NAME,
[ COMMON_BUNDLE_NAME ] ),
Copy link
Member

Choose a reason for hiding this comment

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

IIRC the redundant commons chunk calls here were for the scenario where all included scripts included a reference to a module EXCEPT common.js. In that case, since all scripts reference a module, the module's declaration should be moved to common.js automatically by webpack, instead of leaving them redundantly in each script. I tested this behavior in the old setup and the new setup and it still works as expected 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are correct but in order for that to work you need to set the minChunks property. The default is infinity. You can test this by setting minChunks: 3 and running gulp build. It should increase the size of common.js and decrease the size of each of individual modules. That decrease is due to the standardType being added to common.js as it's required by atleast 3 modules. Do you think we should set the minChunks property?

@anselmbradford
Copy link
Member

LGTM 👍 nice work

Copy link
Member

@ascott1 ascott1 left a comment

Choose a reason for hiding this comment

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

👍 from me as well.

@sebworks sebworks merged commit 13ac0c7 into master Mar 2, 2017
@sebworks sebworks deleted the es6_babel_setup branch March 2, 2017 13:16
@sebworks sebworks mentioned this pull request Mar 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants