Skip to content
This repository has been archived by the owner on Apr 19, 2021. It is now read-only.

Upgrade to Neutrino 9 beta #158

Merged
merged 7 commits into from
Oct 1, 2018
Merged

Upgrade to Neutrino 9 beta #158

merged 7 commits into from
Oct 1, 2018

Conversation

armenzg
Copy link
Contributor

@armenzg armenzg commented Sep 28, 2018

No description provided.

Copy link

@edmorley edmorley left a comment

Choose a reason for hiding this comment

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

Some additional changes:

  • the config.devtool('source-map') can now be replaced by the new built-in devtool web preset option (https://master.neutrinojs.org/packages/web/#source-maps)
  • the web preset's env option now allows defaults to be specified, meaning most of the manual env handling in this file can be replaced (https://master.neutrinojs.org/packages/web/#environment-variables)
  • the manual neutrino.config.output.publicPath('/') can be replaced by the publicPath option to the web preset
  • the links property of the web preset's html option will no longer have any effect (since the underlying template has changed). you'll want to either (a) switch to importing the fonts from an NPM package, (b) copy the default template and add the link there then point at the template using the template option, or (c) find an html-webpack-plugin plugin (see here) that can add them automatically to the existing template

Unrelated to the upgrade but I also spotted these:

warning " > metrics-graphics@2.12.0" has unmet peer dependency "jquery@>=1.11.1".
warning " > react-metrics-graphics@1.0.2" has incorrect peer dependency "react@^0.14.0".
warning " > react-metrics-graphics@1.0.2" has incorrect peer dependency "react-dom@^0.14.0".
warning " > postcss-cssnext@2.11.0" has unmet peer dependency "caniuse-db@^1.0.30000652".

package.json Outdated
"start": "webpack-dev-server --mode development",
"start:local": "BACKEND=http://localhost:3000 webpack-dev-server --mode development",
"test": "jest",
"lint": "eslint --cache --ext mjs,jsx,js src",

Choose a reason for hiding this comment

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

If you'd like tests to be linted too, make this end in src tests rather than just src.
Also for parity with the yarn start / yarn build console output, optionally add --formatter codeframe.

@armenzg
Copy link
Contributor Author

armenzg commented Oct 1, 2018

Hi @edmorley thanks for your help.
I've made some progress on what you mentioned, however, there's a bunch of other issues.

In general I see this warning:

 WARNING in configuration
 The 'mode' option has not been set, webpack will fallback to 'production' for this value. Set 'mode' option to 'development' or 'production' to enable defaults for each environment.
 You can also set it to 'none' to disable any default behavior. Learn more: https://webpack.js.org/concepts/mode/

If I add the 'web' preset I get this failure:

ERROR in ./src/index.jsx
Module build failed (from ./node_modules/babel-loader/lib/index.js):
SyntaxError: /Users/armenzg/repos/health/frontend/src/index.jsx: Unexpected token (18:4)

  16 | const load = () => render(
  17 |   (
> 18 |     <AppContainer>
     |     ^
  19 |       <GenericErrorBoundary critical>
  20 |         <Routes />
  21 |       </GenericErrorBoundary>
    at Parser.raise (/Users/armenzg/repos/health/frontend/node_modules/@babel/parser/lib/index.js:3939:15)
    at Parser.unexpected (/Users/armenzg/repos/health/frontend/node_modules/@babel/parser/lib/index.js:5248:16)
    at Parser.parseExprAtom (/Users/armenzg/repos/health/frontend/node_modules/@babel/parser/lib/index.js:6328:20)
    at Parser.parseExprSubscripts (/Users/armenzg/repos/health/frontend/node_modules/@babel/parser/lib/index.js:5924:21)
...

If I comment out the 'web' preset I get this failure instead:

> ERROR in ./src/index.css
> Module build failed (from ./node_modules/mini-css-extract-plugin/dist/loader.js):
> ModuleBuildError: Module build failed (from ./node_modules/postcss-loader/index.js):
> TypeError: loader._compilation.applyPluginsWaterfall is not a function
>     at /Users/armenzg/repos/health/frontend/node_modules/postcss-loader/index.js:122:43
>     at <anonymous>
>     at runLoaders (/Users/armenzg/repos/health/frontend/node_modules/webpack/lib/NormalModule.js:286:20)
>     at /Users/armenzg/repos/health/frontend/node_modules/loader-runner/lib/LoaderRunner.js:364:11
>     at /Users/armenzg/repos/health/frontend/node_modules/loader-runner/lib/LoaderRunner.js:230:18
>     at context.callback (/Users/armenzg/repos/health/frontend/node_modules/loader-runner/lib/LoaderRunner.js:111:13)
>     at /Users/armenzg/repos/health/frontend/node_modules/postcss-loader/index.js:148:13
>     at <anonymous>
>  @ ./src/index.js 4:0-21
>  @ multi ./src/index
> 
> ERROR in ./node_modules/react-table/react-table.css
> Module build failed (from ./node_modules/mini-css-extract-plugin/dist/loader.js):
> ModuleBuildError: Module build failed (from ./node_modules/postcss-loader/index.js):
> TypeError: loader._compilation.applyPluginsWaterfall is not a function
>     at /Users/armenzg/repos/health/frontend/node_modules/postcss-loader/index.js:122:43
>     at <anonymous>
>     at runLoaders (/Users/armenzg/repos/health/frontend/node_modules/webpack/lib/NormalModule.js:286:20)
>     at /Users/armenzg/repos/health/frontend/node_modules/loader-runner/lib/LoaderRunner.js:364:11
>     at /Users/armenzg/repos/health/frontend/node_modules/loader-runner/lib/LoaderRunner.js:230:18
>     at context.callback (/Users/armenzg/repos/health/frontend/node_modules/loader-runner/lib/LoaderRunner.js:111:13)
>     at /Users/armenzg/repos/health/frontend/node_modules/postcss-loader/index.js:148:13
>     at <anonymous>
>  @ ./src/quantum/flow-table.js 3:0-37
>  @ ./src/routes.js
>  @ ./src/index.js
>  @ multi ./src/index
> 
> ERROR in ./node_modules/metrics-graphics/dist/metricsgraphics.css
> Module build failed (from ./node_modules/mini-css-extract-plugin/dist/loader.js):
> ModuleBuildError: Module build failed (from ./node_modules/postcss-loader/index.js):
> TypeError: loader._compilation.applyPluginsWaterfall is not a function
>     at /Users/armenzg/repos/health/frontend/node_modules/postcss-loader/index.js:122:43
>     at <anonymous>
>     at runLoaders (/Users/armenzg/repos/health/frontend/node_modules/webpack/lib/NormalModule.js:286:20)
>     at /Users/armenzg/repos/health/frontend/node_modules/loader-runner/lib/LoaderRunner.js:364:11
>     at /Users/armenzg/repos/health/frontend/node_modules/loader-runner/lib/LoaderRunner.js:230:18
>     at context.callback (/Users/armenzg/repos/health/frontend/node_modules/loader-runner/lib/LoaderRunner.js:111:13)
>     at /Users/armenzg/repos/health/frontend/node_modules/postcss-loader/index.js:148:13
>     at <anonymous>
>  @ ./node_modules/react-metrics-graphics/build/MetricsGraphics.js 1:4478-4530
>  @ ./src/quantum/subbenchmarks.jsx
>  @ ./src/routes.js
>  @ ./src/index.js
>  @ multi ./src/index
> error Command failed with exit code 2.
> info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

@armenzg
Copy link
Contributor Author

armenzg commented Oct 1, 2018

Also, any info about the favicon?
I'm seeing this in the console:

Failed to load resource: the server responded with a status of 404 (Not Found)

@armenzg
Copy link
Contributor Author

armenzg commented Oct 1, 2018

I'm going to try to drop PostCss since it is always getting on the way.

@armenzg
Copy link
Contributor Author

armenzg commented Oct 1, 2018

@edmorley it seems like order matters:
46dbec2

Should those values be instead within the react preset?

package.json Outdated Show resolved Hide resolved
.neutrinorc.js Outdated Show resolved Hide resolved
@edmorley
Copy link

edmorley commented Oct 1, 2018

In general I see this warning:

That will be fixed by setting mode - eg --mode production. If there's somewhere this can be made clearer in the docs (either the migration guide or the general preset docs), let me know :-)

@edmorley
Copy link

edmorley commented Oct 1, 2018

Also, any info about the favicon?

BREAKING CHANGE @neutrinojs/web, @neutrinojs/node, and their dependent presets no longer configure defaults for copying static files at build time #814. Use the @neutrinojs/copy middleware to configure this for v9.
(https://master.neutrinojs.org/migration-guide/#neutrino-v8-to-v9)

.neutrinorc.js Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
@armenzg armenzg changed the title WIP Upgrade to Neutrino 9 beta Upgrade to Neutrino 9 beta Oct 1, 2018
@armenzg
Copy link
Contributor Author

armenzg commented Oct 1, 2018

Hi @edmorley this is ready to be reviewed. Thanks!

Copy link

@edmorley edmorley left a comment

Choose a reason for hiding this comment

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

I haven't tested locally but the Neutrino-related changes look great!

Before merging I'd recommend also doing one last rm -r yarn.lock node_modules && yarn to refresh the lockfile and pick up latest versions.

package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
.neutrinorc.js Outdated Show resolved Hide resolved
@armenzg
Copy link
Contributor Author

armenzg commented Oct 1, 2018

Thanks for looking!
Sorry you had to look at this code mess! 😢

@armenzg armenzg merged commit 6de378a into master Oct 1, 2018
@armenzg armenzg deleted the neutrino9 branch October 1, 2018 19:42
@edmorley
Copy link

edmorley commented Oct 1, 2018

It was no trouble at all - glad I could help :-)

// Read https://stackoverflow.com/a/36623117
// This is the key to making React Router work with neutrino
// Fix issue with nested routes e.g /index/garbage
publicPath: '/',

Choose a reason for hiding this comment

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

I've been looking into how output.publicPath and devServer.publicPath works for some Neutrino cleanup - and remembered seeing this here. I think this may not be necessary? Reading the linked Stack Overflow issue, the problem is really fixed by using historyApiFallback, which Neutrino has enabled for some time.

Could you try removing it and confirming that both dev and prod builds work as expected? If they don't then it might be a bug that we could fix upstream.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I get this:
image

Choose a reason for hiding this comment

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

Ah I see - I've done some reading up on this area and think Neutrino could be setting better defaults here. I've filed:
neutrinojs/neutrino#1171

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants