Update to Webpack 2.2 #396

julianvmodesto · 2017-02-14T04:58:56Z

Issue

work in progress

julianvmodesto · 2017-02-14T05:42:40Z

Running into eslint errors Parsing error: 'import' and 'export' may only appear at the top level in the starter kyt routes files.

Seems like System.import is gone as of v2.1.0-beta.28, so maybe eslint isn't parsing these ES2015 Loader import()s correctly?

EDIT:

Looks like babel-eslint might be a solution? Or code splitting in starter kyts is blocked until import() is in stage 4 and eslint implements.

This reverts commit c54a3f0.

This reverts commit c2c1e87.

This reverts commit 04df4bf.

tizmagik · 2017-02-14T14:37:18Z

FWIW System.import() will still continue to work, it's just deprecated for now. It'll be removed in Webpack 3 I think they said

julianvmodesto · 2017-02-14T17:11:01Z

Oops, yeah looks like I jumped the gun with replacing System.import(), @tizmagik! Reverting that

delambo · 2017-02-14T18:57:40Z

packages/kyt-starter-universal/.babelrc

@@ -1,3 +1,4 @@
 {
-  "presets": ["kyt-react"]
+  "presets": ["kyt-react"],
+  "plugins": ["syntax-dynamic-import"]


Do you think we should just add this plugin to our babel-preset-kyt-core? This seems like something kyt should support/recommend out of the box.

delambo · 2017-02-14T19:06:58Z

@julianvmodesto - I think we should keep your System.import changes. Can you try babel-eslint in our linter?

tizmagik · 2017-02-14T19:09:18Z

Yea agreed, it would be nice to upgrade to the new import() syntax, if possible.

julianvmodesto · 2017-02-14T19:15:42Z

Sure thing, I'll make these changes –

move babel-plugin-syntax-dynamic-import to babel-preset-kyt-core's devDeps
try out babel-eslint in babel-preset-kyt-core

julianvmodesto · 2017-02-15T06:06:19Z

Seems that the Travis build failed since bootstrap.js was expecting yarn 0.19.0 but instead the latest yarn version 0.20.3 was installed.

0.20.3
❌  update your version of yarn: npm i yarn@^0.19.0 -g

Not sure how Travis works, but I think we want in the build script:

curl -o- -L https://yarnpkg.com/install.sh | bash -s -- --version 0.19.0

EDIT:
Travis takes care of Yarn for us, so instead we want an update to bootstrap.js for yarn >=0.19.0: #400

julianvmodesto · 2017-02-16T21:07:25Z

packages/babel-preset-kyt-core/lib/index.js

@@ -1,15 +1,26 @@
 var babelPresetLatest = require('babel-preset-latest');
 var babelTransformRuntime = require('babel-plugin-transform-runtime');
 var babelTransformModules = require('babel-plugin-transform-es2015-modules-commonjs');
+var babelSyntaxDynamicImport = require('babel-plugin-syntax-dynamic-import');
+var babelEslint = require('babel-eslint');


I'm trying to add babel-eslint here in packages/babel-preset-kyt-core, but kyt itself is linted with packages/eslint-config-kyt – so I'm guessing I have to add babel-eslint and babel-plugin-syntax-dynamic-import to eslint-config-kyt as well.

This reverts commit 16e69cd.

julianvmodesto · 2017-02-17T04:54:13Z

Ok, looks like we're good on the System.import to import() change and babel-eslint is working correctly - thanks Matt!

Currently troubleshooting an error with the webpackCompiler:


$ kyt dev 
ℹ️  Using kyt config at /home/travis/build/NYTimes/kyt/cli-test/kyt.config.js
🔥  Starting development build...
👍  Cleaned ./build
❌  Client webpack config is invalid
TypeError: arguments[i].apply is not a function
    at Compiler.apply (/home/travis/build/NYTimes/kyt/cli-test/node_modules/tapable/lib/Tapable.js:306:16)
    at webpack (/home/travis/build/NYTimes/kyt/cli-test/node_modules/webpack/lib/webpack.js:32:19)
    at module.exports.error (/home/travis/build/NYTimes/kyt/cli-test/node_modules/kyt/utils/webpackCompiler.js:10:23)
    at module.exports (/home/travis/build/NYTimes/kyt/cli-test/node_modules/kyt/cli/actions/dev.js:68:20)
    at loadConfigAndDo (/home/travis/build/NYTimes/kyt/cli-test/node_modules/kyt/cli/commands.js:34:3)
    at Command.program.command.option.description.action (/home/travis/build/NYTimes/kyt/cli-test/node_modules/kyt/cli/commands.js:49:5)
    at Command.listener (/home/travis/build/NYTimes/kyt/cli-test/node_modules/commander/index.js:301:8)
    at emitTwo (events.js:106:13)
    at Command.emit (events.js:191:7)
    at Command.parseArgs (/home/travis/build/NYTimes/kyt/cli-test/node_modules/commander/index.js:615:12)

I tried checking out what Tapable is doing, and it's running apply from the plugin prototype, but none of the plugins have this apply method in their prototype.

Googling around for this error and lewie9021/webpack-configurator#2 seems a little insightful – I'm putting some console.log statements in Tapable to see what's happening. Will update as I go along. If anyone has ideas, please let me know! Maybe there's some Webpack 2.2 change that I'm missing.

uncomment

fix lint error

delambo · 2017-02-23T02:47:50Z

@julianvmodesto - ci test is passing again! I think we should merge #405 before this pull request so that we can run some of the new functional tests against the new webpack and loaders.

What do you think is left to do here?

delambo · 2017-02-23T20:19:39Z

@julianvmodesto - I got the new functional tests merged in. Looks good 👍

julianvmodesto · 2017-02-23T21:27:08Z

Ah great! I don't have anything else to add I think. Merge onwards?

delambo · 2017-02-25T17:38:24Z

@jaredmcdonald @ccpricenytimes @tizmagik - Does anyone want to take a look at this? I tested it against our internal app. Looks good.

ccpricenytimes

This is blocked by #399

tizmagik

This is awesome!! 👏

jaredpalmer · 2017-02-28T15:30:50Z

What is the status of this?

julianvmodesto · 2017-03-01T03:02:48Z

Currently blocked by #399, but otherwise all set

just why

delambo · 2017-03-08T22:43:52Z

Thank you @julianvmodesto 🙏

* master: Upgrade stylelint (#417) Update to Webpack 2.2 (#396) Version and install starter-kyts with npm (#425) adds prep for 0.4.1 (#428) fixes static public path for noServer configuration (#427) Adds local-path argument for testing local starter-kyt setup (#403) # Conflicts: # packages/kyt-core/package.json # packages/kyt-core/yarn.lock # packages/kyt-starter-static/starter-src/yarn.lock # packages/kyt-starter-universal/starter-src/src/yarn.lock # yarn.lock

julianvmodesto added 6 commits February 13, 2017 22:56

Upgrade yarn.locks

248d8b2

First (rough) pass, tests broked 💩

5d2e1ee

Update yarn.locks

d6331a2

Try suppressing eslint errors

04df4bf

Upgrade eslint

c2c1e87

Try updating eslint-plugin-import

c54a3f0

julianvmodesto added 5 commits February 14, 2017 00:52

Revert "Try updating eslint-plugin-import"

5e46f40

This reverts commit c54a3f0.

Revert "Upgrade eslint"

9e8d9de

This reverts commit c2c1e87.

Revert "Try suppressing eslint errors"

1bc7339

This reverts commit 04df4bf.

Update yarn.locks

877c599

Make sure babel-plugin-syntax-dynamic-import is not a devDep

5fe60b3

delambo reviewed Feb 14, 2017

View reviewed changes

Try babel-eslint + syntax-dynamic-import in babel-preset-kyt-core

a81a8c4

julianvmodesto added 2 commits February 15, 2017 01:36

Improve readability

04fd2ba

Expand version range for yarn so Travis' yarn works

052f3b0

julianvmodesto commented Feb 16, 2017

View reviewed changes

julianvmodesto added 5 commits February 16, 2017 22:43

Put babel-eslint in the right place (maybe)

e4fc3de

Merge branch 'master' into update-webpack-2.2

e631a41

Try finding babel-eslint peer dep

16e69cd

Revert "Try finding babel-eslint peer dep"

1b05fac

This reverts commit 16e69cd.

Update webpack-dev deps

c372741

Update webpack-merge

7d53d97

julianvmodesto and others added 4 commits February 21, 2017 21:03

📌 pin deps

a1d63d7

📌 pin deps in starter kyts

3de2eef

Bump more loaders

c430f9b

fixes e2e cli test

c179995

uncomment

delambo force-pushed the update-webpack-2.2 branch from 404857b to c179995 Compare February 23, 2017 01:48

delambo added 2 commits February 22, 2017 20:52

Merge branch 'master' into update-webpack-2.2

9ccc446

fix lint error

8c7f62a

fix lint error

delambo force-pushed the update-webpack-2.2 branch from 40ac3e3 to 8c7f62a Compare February 23, 2017 02:00

fix path

4182b07

delambo changed the title ~~Update to Webpack 2.2~~ [WIP] Update to Webpack 2.2 Feb 23, 2017

delambo added 2 commits February 23, 2017 15:08

Merge branch 'master' into update-webpack-2.2

57cc5e7

fix lint errors from merge conflicts

b23ed80

delambo changed the title ~~[WIP] Update to Webpack 2.2~~ Update to Webpack 2.2 Feb 25, 2017

delambo approved these changes Feb 25, 2017

View reviewed changes

ccpricenytimes previously requested changes Feb 25, 2017

View reviewed changes

tizmagik approved these changes Feb 25, 2017

View reviewed changes

Fix fallback

720ece1

delambo added 2 commits March 3, 2017 20:56

merge upstream master

a658b19

merge upstream master

0475a6d

delambo merged commit 57c38d7 into nytimes:master Mar 8, 2017

julianvmodesto deleted the update-webpack-2.2 branch March 8, 2017 23:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to Webpack 2.2 #396

Update to Webpack 2.2 #396

julianvmodesto commented Feb 14, 2017 •

edited

julianvmodesto commented Feb 14, 2017 •

edited

tizmagik commented Feb 14, 2017

julianvmodesto commented Feb 14, 2017

delambo Feb 14, 2017

delambo commented Feb 14, 2017

tizmagik commented Feb 14, 2017

julianvmodesto commented Feb 14, 2017

julianvmodesto commented Feb 15, 2017 •

edited

julianvmodesto Feb 16, 2017

julianvmodesto commented Feb 17, 2017 •

edited

delambo commented Feb 23, 2017

delambo commented Feb 23, 2017

julianvmodesto commented Feb 23, 2017

delambo commented Feb 25, 2017

ccpricenytimes left a comment

tizmagik left a comment

jaredpalmer commented Feb 28, 2017

julianvmodesto commented Mar 1, 2017

delambo commented Mar 8, 2017

Update to Webpack 2.2 #396

Update to Webpack 2.2 #396

Conversation

julianvmodesto commented Feb 14, 2017 • edited

Issue

julianvmodesto commented Feb 14, 2017 • edited

tizmagik commented Feb 14, 2017

julianvmodesto commented Feb 14, 2017

delambo Feb 14, 2017

Choose a reason for hiding this comment

delambo commented Feb 14, 2017

tizmagik commented Feb 14, 2017

julianvmodesto commented Feb 14, 2017

julianvmodesto commented Feb 15, 2017 • edited

julianvmodesto Feb 16, 2017

Choose a reason for hiding this comment

julianvmodesto commented Feb 17, 2017 • edited

delambo commented Feb 23, 2017

delambo commented Feb 23, 2017

julianvmodesto commented Feb 23, 2017

delambo commented Feb 25, 2017

ccpricenytimes left a comment

Choose a reason for hiding this comment

tizmagik left a comment

Choose a reason for hiding this comment

jaredpalmer commented Feb 28, 2017

julianvmodesto commented Mar 1, 2017

delambo commented Mar 8, 2017

julianvmodesto commented Feb 14, 2017 •

edited

julianvmodesto commented Feb 14, 2017 •

edited

julianvmodesto commented Feb 15, 2017 •

edited

julianvmodesto commented Feb 17, 2017 •

edited