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

Upgrade babel, webpack, and node #1687

Merged
merged 4 commits into from Apr 2, 2018

Conversation

Projects
None yet
2 participants
@jseppi
Contributor

jseppi commented Mar 30, 2018

open against #1685 currently

Upgrades us to babel 6.26, webpack 4.4, and node 6.13. The node upgrade was required in order to work with webpack 4.4.

Also switches us to babel-preset-env instead of babel-preset-es2015, which is deprecated.

TODO:

  • figure out why test is failing
    • it was because the new webpack needs node >= v6.11
  • do some manual testing/investigation to make sure everything looks good

James Seppi added some commits Mar 30, 2018

James Seppi
upgrade babel and webpack
also switch to babel-preset-env, which will target the latest 2 browser versions
James Seppi

@jseppi jseppi changed the title from [WIP] Upgrade babel and webpack to [WIP] Upgrade babel, webpack, and node Mar 30, 2018

James Seppi

@jseppi jseppi changed the title from [WIP] Upgrade babel, webpack, and node to Upgrade babel, webpack, and node Mar 30, 2018

@jseppi jseppi requested a review from toolness Mar 30, 2018

@jseppi jseppi self-assigned this Mar 30, 2018

@@ -1,3 +1,7 @@
/**
* @jest-environment node

This comment has been minimized.

@jseppi

jseppi Mar 30, 2018

Contributor

required, otherwise jest uses jsdom, which doesn't have the same setTimeout capabilities, apparently.

This comment has been minimized.

@toolness

toolness Apr 2, 2018

Contributor

Heh, I learned too, this while using TypeScript and expecting setTimeout() to return a number but instead getting this funky Timeout class. Wacky!

@toolness

Looks good to me, thanks!

return webpackStream({
mode: isProd ? 'production' : 'development',

This comment has been minimized.

@toolness

toolness Apr 2, 2018

Contributor

Interesting, I read up on mode and it looks like it might allow us to get rid of more of our code that branches based on isProd:

  • Apparently process.env.NODE_ENV is now automatically set to either "development" or "production" based on mode. While we currently use the DefinePlugin to set it to "" or "production", we might want to consider removing that logic entirely, or adding a TODO that mentions it may be redundant.

  • mode has its own defaults for devtool, but our overrides are different and seem to work well for us, so I don't think we should worry about it.

This comment has been minimized.

@jseppi

jseppi Apr 2, 2018

Contributor

Good catch! Just removed DefinePlugin and made a minor adjustment to our webpack tests.

@jseppi jseppi force-pushed the upgrade-webpack branch from 2398ee9 to 6112503 Apr 2, 2018

James Seppi
rm use of DefinePlugin
Webpack v4's `mode` handles setting `process.env.NODE_ENV` based on the value of `mode`
so we do not need to do it manually via the `DefinePlugin`.

@jseppi jseppi merged commit 3af03a1 into upgrade-react-v16 Apr 2, 2018

1 check passed

ci/circleci: build Your tests passed on CircleCI!
Details

@jseppi jseppi deleted the upgrade-webpack branch Apr 2, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment