Webpack 4 #4077
Webpack 4 #4077
Conversation
| @@ -18,10 +18,11 @@ class ModuleScopePlugin { | |||
|
|
|||
| apply(resolver) { | |||
| const { appSrc } = this; | |||
| resolver.plugin('file', (request, callback) => { | |||
| resolver.hooks.file.tap('ModuleScopePlugin', (request, callback) => { | |||
There was a problem hiding this comment.
For now you can reference the documentation of tapable. I believe you want to use tapAsync in this case.
There was a problem hiding this comment.
Thanks @simonkberg! While thinking about it, since the plugin isnt doing any async should be valid to use .tap, return early where possible then just throw the error instead of passing it to the callback? What do you think?
packages/react-dev-utils/README.md
Outdated
| // Generates an `index.html` file with the <script> injected. | ||
| new HtmlWebpackPlugin({ | ||
| inject: true, | ||
| template: path.resolve('public/index.html'), | ||
| }), | ||
| // ... | ||
| ], | ||
| // Makes the public URL available as %PUBLIC_URL% in index.html, e.g.: |
There was a problem hiding this comment.
Moved below HtmlWebpackPlugin otherwise the tapable hooks are not available for tapping
There was a problem hiding this comment.
This is down one line too far outside the closing bracket for plugins:[...]
There was a problem hiding this comment.
@thephw thanks, fixed!
packages/react-scripts/package.json
Outdated
| "uglifyjs-webpack-plugin": "1.1.6", | ||
| "sw-precache-webpack-plugin": "goldhand/sw-precache-webpack-plugin#master", | ||
| "thread-loader": "1.1.4", | ||
| "uglifyjs-webpack-plugin": "1.2.2", |
There was a problem hiding this comment.
i think we should just use the built-in uglifyjs now that we use webpack 4 https://github.com/webpack-contrib/uglifyjs-webpack-plugin#uglifyjs-webpack-plugin
There was a problem hiding this comment.
I agree but the info on the link is obsolete. (Error: webpack.optimize.UglifyJsPlugin has been removed, please use config.optimization.minimize instead.)
I will move it to config.optimization when the webpack 4 doc is released.
There was a problem hiding this comment.
i see, thanks for the info!
| @@ -18,10 +18,11 @@ class ModuleScopePlugin { | |||
|
|
|||
| apply(resolver) { | |||
| const { appSrc } = this; | |||
| resolver.plugin('file', (request, callback) => { | |||
| resolver.hooks.file.tap('ModuleScopePlugin', (request, callback) => { | |||
There was a problem hiding this comment.
For now you can reference the documentation of tapable. I believe you want to use tapAsync in this case.
andriijas
force-pushed
the
webpack4
branch
3 times, most recently
from
670ecd9
to
3d93efd
Compare
|
@andriijas it looks like you're upgrading a lot of dependencies in this PR unrelated to Webpack, what's the reason for that? |
|
I’m thinking you might want to in order to keep the noise low in the diff
and make it easier to review. If you want to bump versions, I’d do that in
a separate PR.
…On Wed, Feb 28, 2018 at 4:17 PM Andreas Cederström ***@***.***> wrote:
@pmdarrow <https://github.com/pmdarrow> actually no particular other than
just bumping. i can revert everything except plugins and loaders if you
want to?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#4077 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAj0MOWJ6kod_aOWfPWvpmwjJQnj_uFcks5tZbRNgaJpZM4SS3n5>
.
|
Yes, definitely :-) Now, I have app which should support SSR (Server Side Rendering). In addition I propose to think forward.. You for example can create one Example of my current webpack.config.js:const getEnvironment = require('./env');
const rawEnv = getEnvironment().raw;
const isDevelopment = rawEnv.NODE_ENV === 'development';
const isProduction = rawEnv.NODE_ENV === 'production';
if (!(isDevelopment || isProduction)) {
throw new Error(
'The NODE_ENV environment variable must be "development" or "production".'
);
}
// https://webpack.js.org/configuration/configuration-types/#exporting-multiple-configurations
module.exports = [
(isProduction
? require('./webpack.config.client.prod')
: require('./webpack.config.client.dev')
),
(isProduction
? require('./webpack.config.server.prod')
: require('./webpack.config.server.dev')
),
];Thanks for the hard work! ❤️ |
update
@pmdarrow actually i updated to try to pin point which dependency that spits out but i havent managed to figure out which dependency still uses the old webpack plugin api. @langpavel Looking at the configs in CRA i think there are equally many differences as similarities. Merging the files, there will be a lot of branching/if statements checking which env we are targeting. |
|
|
|
nice job @andriijas 🥇 FYI from my mirror of your analysis (things I figured out before I found your PR);
both warnings seem to stem from The warnings are emitted via package Reproducible with at least one plugin my 2 pennies |
|
@AubreyHewes you are right, while html-webpack-plugin added support for the new webpack plugin api they didnt remove/upgrade all deprecated calls to compiler.plugin. |
|
FWIW, the associated issue on |
andriijas
force-pushed
the
webpack4
branch
2 times, most recently
from
c3436d1
to
cd0f7f7
Compare
Update 6/3/18
|
|
@andriijas let me see if I can help out :) I've cloned your branch and will see what I find. |
|
@andriijas I may have done merge commit wrong PR'ing into your branch, but let's see how this turns out. andriijas#2 |
|
This is just resolving conflicts and running CI again. Is there any way to run kitchen sink tests locally? |
|
Thx @TheLarkInn - I'm spending the day in the car. It should be possible. Eventually need to install node 8 to reproduce the specific error locally. Not sure. The merge commit might be because I've yet to rebase the latest changes from next. We can resolve that later. |
|
@TheLarkInn Unfortunately the e2e tests don't work reliably when run locally. They install some global packages and mess with your yarn and npm settings so you probably don't want to run them on your machine. There's a Docker version that someone contributed but that's also broken right now. 😞 |
|
So I ran Node 8 and didn't see this problem crop up from running both dev and production builds. If anyone has a few repro steps it will make patching manifest-webpack-plugin (if needed) way easier. |
|
If I'd have to guess something is screwy with |
|
@TheLarkInn I was able to get a repro. [--- EDIT: this PR was merged to the create-react-app's next branch ---] and run "yarn e2e:docker --interactive --test-suite kitchensink" |
|
Jack, thanks a lot! This should give me something to work with. I'll report
back once I take a crack at it tonight.
…On Sun, May 13, 2018, 12:16 PM Jack Zhao ***@***.***> wrote:
@TheLarkInn <https://github.com/TheLarkInn> I was able to get a repro.
You'll need to apply the following patch:
***@***.***
<bugzpodder/create-react-app@9a5bd5d>
and run "yarn e2e:docker --interactive --test-suite kitchensink"
$ react-scripts build
Creating an optimized production build...
WARNING: The `text-hide()` mixin has been deprecated as of v4.1.0. It will be removed entirely in v5.
on line 10 of node_modules/bootstrap/scss/mixins/_text-hide.scss, in mixin `text-hide`
from line 57 of node_modules/bootstrap/scss/utilities/_text.scss
from line 14 of node_modules/bootstrap/scss/_utilities.scss
from line 41 of node_modules/bootstrap/scss/bootstrap.scss
from line 1 of stdin
path.js:7
throw new TypeError('Path must be a string. Received ' + inspect(path));
^
TypeError: Path must be a string. Received undefined
at assertPath (path.js:7:11)
at Object.basename (path.js:1361:5)
at moduleAsset (/tmp/tmp.tgGgDwkWWZ/test-kitchensink/node_modules/webpack-manifest-plugin/lib/plugin.js:45:12)
at SyncHook.eval (eval at create (/tmp/tmp.tgGgDwkWWZ/test-kitchensink/node_modules/tapable/lib/HookCodeFactory.js:51:12), <anonymous>:7:1)
at Compilation.createModuleAssets (/tmp/tmp.tgGgDwkWWZ/test-kitchensink/node_modules/webpack/lib/Compilation.js:1768:29)
at hooks.optimizeTree.callAsync.err (/tmp/tmp.tgGgDwkWWZ/test-kitchensink/node_modules/webpack/lib/Compilation.js:939:9)
at AsyncSeriesHook.eval [as callAsync] (eval at create (/tmp/tmp.tgGgDwkWWZ/test-kitchensink/node_modules/tapable/lib/HookCodeFactory.js:24:12), <anonymous>:6:1)
at AsyncSeriesHook.lazyCompileHook [as _callAsync] (/tmp/tmp.tgGgDwkWWZ/test-kitchensink/node_modules/tapable/lib/Hook.js:35:21)
at Compilation.seal (/tmp/tmp.tgGgDwkWWZ/test-kitchensink/node_modules/webpack/lib/Compilation.js:890:27)
at hooks.make.callAsync.err (/tmp/tmp.tgGgDwkWWZ/test-kitchensink/node_modules/webpack/lib/Compiler.js:481:17)
at _done (eval at create (/tmp/tmp.tgGgDwkWWZ/test-kitchensink/node_modules/tapable/lib/HookCodeFactory.js:24:12), <anonymous>:9:1)
at _err1 (eval at create (/tmp/tmp.tgGgDwkWWZ/test-kitchensink/node_modules/tapable/lib/HookCodeFactory.js:24:12), <anonymous>:32:22)
at _addModuleChain (/tmp/tmp.tgGgDwkWWZ/test-kitchensink/node_modules/webpack/lib/Compilation.js:758:12)
at processModuleDependencies.err (/tmp/tmp.tgGgDwkWWZ/test-kitchensink/node_modules/webpack/lib/Compilation.js:697:9)
at _combinedTickCallback (internal/process/next_tick.js:73:7)
at process._tickCallback (internal/process/next_tick.js:104:9)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
++ set +x
e2e-kitchensink.sh: ERROR! An error was encountered executing line 128.
Cleaning up.
yarn config v1.6.0
success Set "registry" to "https://registry.yarnpkg.com".
Done in 0.09s.
Exiting with error.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#4077 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ADQBMFKf1o41yFnjsr6wurxBCVVOwUxoks5tyIaCgaJpZM4SS3n5>
.
|
|
Just a little thing I've seen in playing with this - and I don't know if this is a webpack 4 thing, a more general CRA 2.0 thing, or something I've done - but I've found that importing libraries that bundle both frontend and node code seems to pick the node version in some cases. I've found this problem with both firebase and socket.io, but I think there may be others. |
|
@DanProudfoot File a new issue pls? |
- [x] Utilize webpack 4 development and production modes - [x] Upgrade webpack dev server - [x] Webpack 4 compatible release of thread-loader - [x] Webpack 4 compatible release of HtmlWebpackPlugin - [x] Webpack 4 compatible release of SwPrecacheWebpackPlugin - [x] Webpack 4 compatible release of WebpackManifestPlugin - [x] Update README - [x] Update WebpackDevServerUtils - [x] Update InterpolateHtmlPlugin - [x] Update ModuleScopePlugin - [x] Update WatchMissingNodeModulesPlugin - [x] Move UglifyJS options to webpack 4 optimize - [x] Move InterpolateHtmlPlugin to make it tapable on HtmlWebpackPlugin - [x] vendor splitting via splitChunks.splitChunks (https://twitter.com/wSokra/status/969633336732905474) - [x] long term caching via splitChunks.runtimeChunk (https://twitter.com/wSokra/status/969679223278505985) - [x] Make sure process.env.NODE_ENV is proxied correctly to `react-error-overlay` - [x] Implicit webpack.NamedModulesPlugin in dev config as its default in webpack 4 - [x] Disable webpack performance hints as we have our own filesize reporter - [x] Replace ExtractTextPlugin with MiniCssExtractPlugin - [x] Switch to css whole file minification via OptimizeCSSAssetsPlugin rather than per module css minification to gain performance
|
@bugzpodder i dont have docker available right now - can you debug why the manifest gets path undefined? thanks 🥂 |
|
Sorry all, it was my own stupid fault - I was using directory-named-webpack-plugin, and that breaks things if I don't set Sorry all! |
|
Did a bit of debugging. For the actual manifest plugin issue from webpack build,
|
|
|
I filed #4492 for follow-up discussion. |
|
Great job! 🎉 |
|
Well done, guys! |
|
GG @andriijas |
|
As usual for 2.x alphas, we'll post about new releases in #3815. I'm locking this one to reduce the notification noise. Thanks again to everyone! |
create-react-apppolyfills included in vendor chunkprocess.env.NODE_ENVfrom ancestorFixes: #3878