Migrate tc-web to neutrino v9 #157

helfi92 · 2018-10-10T20:18:54Z

Fixes #154.

edmorley · 2018-10-11T14:45:12Z

.neutrinorc.js

+            legacyDecorators: true
+          }
+        },
+        emitWarning: process.env.NODE_ENV === 'development',


We'll likely be making this change upstream soon, see:
neutrinojs/neutrino#1131 (comment)

edmorley · 2018-10-11T14:50:04Z

.neutrinorc.js

-      'APPLICATION_NAME',
-      'LOGIN_STRATEGIES',
-      'TASKCLUSTER_ROOT_URL',
-    ]],
    (neutrino) => {
      if (process.env.NODE_ENV === 'development') {
        neutrino.config.devtool('cheap-module-source-map');


I'd move this to the devtool option within @neutrinojs/react, since it ensures terser-webpack-plugin will be configured correctly (switching to terser is currently unreleased; see neutrinojs/neutrino#1158).

Also, the default is cheap-module-eval-source-map, which is likely faster for incremental builds (given eval), so unless there is some issue being encountered, might be best left at the default. See:
https://webpack.js.org/configuration/devtool/#devtool

edmorley · 2018-10-11T14:50:49Z

.neutrinorc.js

@@ -79,6 +168,10 @@ module.exports = {
          bindings: 'bindings'
        }));

+      // TODO: Why is this needed?
+      neutrino.config.resolve.modules.add(MODULES);


I would recommend not adding this. If not having it causes an error, could you include it in the code comment? (And I'm happy to take a look)

edmorley · 2018-10-11T14:52:26Z

.neutrinorc.js

@@ -96,6 +189,8 @@ module.exports = {
          .test(/JSONStream/)
          .use('shebang')
            .loader('shebang-loader');
+
+      console.log(JSON.stringify(neutrino.config.toConfig(), null, 2));


Manually stringifying the config is not neccessary/gives worse output. Try:

neutrino --inspect --mode {development,production} # or NODE_ENV={development,production,test} neutrino --inspect

(https://master.neutrinojs.org/usage/#inspecting-the-generated-webpack-config)

➜ taskcluster-web git:(neutrino-v9) ✗ neutrino --inspect --mode development /Users/haali/Documents/Mozilla/projects/neutrino-dev/node_modules/webpack-chain/src/Plugin.js:11 this.init((Plugin, args = []) => new Plugin(...args)); ^ TypeError: Plugin is not a constructor at init (/Users/haali/Documents/Mozilla/projects/neutrino-dev/node_modules/webpack-chain/src/Plugin.js:11:40) at Object.toConfig (/Users/haali/Documents/Mozilla/projects/neutrino-dev/node_modules/webpack-chain/src/Plugin.js:42:22) at clean.Object.assign.plugins.plugins.values.map.plugin (/Users/haali/Documents/Mozilla/projects/neutrino-dev/node_modules/webpack-chain/src/Config.js:125:61) at Array.map (<anonymous>) at module.exports.toConfig (/Users/haali/Documents/Mozilla/projects/neutrino-dev/node_modules/webpack-chain/src/Config.js:125:40) at module.exports.toString (/Users/haali/Documents/Mozilla/projects/neutrino-dev/node_modules/webpack-chain/src/Config.js:137:41) at Object.output (/Users/haali/Documents/Mozilla/projects/neutrino-dev/packages/neutrino/index.js:27:37) at Object.<anonymous> (/Users/haali/Documents/Mozilla/projects/neutrino-dev/packages/neutrino/bin/neutrino.js:9:20) at Module._compile (module.js:652:30) at Object.Module._extensions..js (module.js:663:10)

It looks like the globally yarn linked version of Neutrino is using an older version of webpack-chain. I would either:

run yarn neutrino ... (in the hope that it favours the copy of Neutrino in the local node_modules)

remove the yarn link for neutrino

update the neutrino-dev source directory to master and run yarn to install the latest version of deps

(I would suggest 1/2, otherwise we'll be debugging Neutrino master rather than the known release version, which is not what yarn start will be using)

Created neutrinojs/neutrino#1167.

I'm not sure I am running a linked version of Neutrino. Running grep "webpack-chain" node_modules/neutrino/package.json shows "webpack-chain": "^4.11.0",. The update to webpack-chain was merged two days ago. Is it possible that it hasn't been published yet?

If neutrino is on PATH (ie neutrino works and not just yarn neutrino), it's a linked copy, or else PATH has been manually adjusted.

That grep command is a red herring, since that will be grepping the local node_modules copy, not the copy that's being used from the linked neutrino.

Try running this:
ls -al $(yarn global bin)/neutrino

Removing the linked version of Neutrino works! Many thanks!

Awesome :-)

edmorley · 2018-10-11T14:52:39Z

karma.conf.js

+
+process.env.NODE_ENV = process.env.NODE_ENV || 'test';
+
+module.exports = neutrino().karma();


(Missing end of file newline)

edmorley · 2018-10-11T14:53:59Z

.neutrinorc.js

-          'react/no-did-mount-set-state': 'off',
-        }
-      }
+      env: [


If it helps, this env option now also supports the object-form that can take defaults for each variable.

edmorley · 2018-10-11T14:55:38Z

.neutrinorc.js

+      // TODO: Why is this needed?
+      neutrino.config.resolve.modules.add(MODULES);
+      // neutrino.config.resolveLoader.modules.add(MODULES);
+
      neutrino.config.output.publicPath('/');


This can be set via the @neutrinojs/react options now, which will also ensure the devserver equivalent option is set to a matching value automatically.

helfi92 · 2018-10-12T14:31:06Z

Blocked by neutrinojs/neutrino#1166.

edmorley · 2018-10-14T20:32:40Z

package.json

+    "build": "yarn create:fragment-matcher && dotenv webpack -- --mode production",
+    "start": "dotenv webpack-dev-server -- --mode development",
+    "test": "karma start --single-run",
+    "lint": "eslint --cache --format codeframe --ext mjs,js src test",


This needs to be --ext mjs,jsx,js

Wow, that was silly from my end. Sorry for that.

I didn't see it the first time I looked either 😆

helfi92 · 2018-10-16T17:58:35Z

OK, I think this is ready. I'll merge after #160 if there are no objections.

/cc @djmitche.

djmitche · 2018-10-16T18:10:12Z

None from me!

edmorley

lgtm :-)

helfi92 self-assigned this Oct 10, 2018

helfi92 force-pushed the neutrino-v9 branch from a974bc5 to 5526044 Compare October 11, 2018 14:16

helfi92 mentioned this pull request Oct 11, 2018

Linting issue with neutrino v9 neutrinojs/neutrino#1166

Closed

edmorley reviewed Oct 11, 2018

View reviewed changes

edmorley mentioned this pull request Oct 11, 2018

neutrino --inspect throws an error neutrinojs/neutrino#1167

Closed

edmorley reviewed Oct 14, 2018

View reviewed changes

helfi92 force-pushed the neutrino-v9 branch from 586435c to 78ec190 Compare October 15, 2018 16:40

helfi92 changed the title ~~[WIP] Migrate tc-web to neutrino v9~~ Migrate tc-web to neutrino v9 Oct 15, 2018

helfi92 force-pushed the neutrino-v9 branch 2 times, most recently from 270c712 to 765547f Compare October 16, 2018 17:45

edmorley reviewed Oct 16, 2018

View reviewed changes

helfi92 added 9 commits October 17, 2018 10:35

WIP Migrate tc-web to neutrino v9

777d286

More changes

61e0b9b

Add requested changes

5b78027

Nits

07c527f

Use material's new typograpghy v2 variants

2775e83

Fix linting errors

a546405

Add required changes after rebasing

48d735d

Regenerate yarn.lock with node 10

079d3c8

Nits after rebasing on master

bf98c37

helfi92 force-pushed the neutrino-v9 branch from ad47409 to bf98c37 Compare October 17, 2018 15:21

helfi92 merged commit 0a187e3 into taskcluster:master Oct 17, 2018

helfi92 mentioned this pull request Oct 17, 2018

Add ability to close all error panels #118

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate tc-web to neutrino v9 #157

Migrate tc-web to neutrino v9 #157

helfi92 commented Oct 10, 2018 •

edited

edmorley Oct 11, 2018

edmorley Oct 11, 2018

edmorley Oct 11, 2018

edmorley Oct 11, 2018 •

edited

helfi92 Oct 11, 2018

edmorley Oct 11, 2018

helfi92 Oct 11, 2018

helfi92 Oct 11, 2018

edmorley Oct 11, 2018

helfi92 Oct 11, 2018

edmorley Oct 11, 2018

edmorley Oct 11, 2018

edmorley Oct 11, 2018

edmorley Oct 11, 2018

helfi92 commented Oct 12, 2018

edmorley Oct 14, 2018

helfi92 Oct 15, 2018

edmorley Oct 15, 2018

helfi92 commented Oct 16, 2018

djmitche commented Oct 16, 2018

edmorley left a comment


		process.env.NODE_ENV = process.env.NODE_ENV \|\| 'test';

		module.exports = neutrino().karma();

Migrate tc-web to neutrino v9 #157

Migrate tc-web to neutrino v9 #157

Conversation

helfi92 commented Oct 10, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edmorley Oct 11, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

helfi92 commented Oct 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

helfi92 commented Oct 16, 2018

djmitche commented Oct 16, 2018

edmorley left a comment

Choose a reason for hiding this comment

helfi92 commented Oct 10, 2018 •

edited

edmorley Oct 11, 2018 •

edited