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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

webpack 5 alpha feedback #8537

Open
sokra opened this Issue Dec 21, 2018 · 84 comments

Comments

Projects
None yet
@sokra
Copy link
Member

sokra commented Dec 21, 2018

馃巹 First, thanks for trying and giving feedback to our alpha version. 馃巹

Note that it's still in an early state and stuff may be broken. This is something you want to report here, as we don't want this to happen in the final release.

Note that the major version has breaking changes and some plugins might no longer work as expected. We try to provided compatibility layers when possible, but some of the changes make this difficult (especially regarding injection additional runtime code). If some plugin isn't working this is also something you should report here to we are at least aware of it and may work on fixing it or adding a compatibility layer.

Here is the full Changelog. Make sure to read at least until the configuration changes to be up-to-date.

If you find something missing in the changelog, this is also something to report here. Easy fixes can made directly in the repo. Every webpack contributor has write access, otherwise send a PR.

If everything is working great, that's also nice to hear. If you are trying the Persistent Caching feature, we would love to see numbers.

Please make sure to try with the lastest alpha version before reporting, as it may already be fixed. You can use the npm dist-tag next to reference the latest alpha version.

package.json "webpack": "next". Update with yarn upgrade webpack / npm update webpack.

Not to forget: Thanks for everybody contributed to this version (@byzyk, @cacheflow, @Connormiha, @fscherwi, @guybedford, @michael-ciniawsky, @niieani, @ooflorent, @SevInf, @smelukov, @sodatea, @sokra, @xtuc). 馃帀


Feedback summary:

  • mini-css-extract-plugin is incompatible (RuntimeModules change)
  • fork-ts-checker-webpack-plugin is incompatible (frozen hooks)
  • circular-dependency-plugin is incompatible (dep.module removed)
  • cache-loader is no longer needed
  • webpack-cli misses Stats.presetToOptions (should use compilation.createStatsOptions)
  • html-webpack-plugin misses fileTimestamps (should use fileSystemInfo)
  • null prototype objects can't be serialized
  • Deprecation warning for the HMR defaults
  • Docs need update
  • namedChunkGroups have weird keys
  • MultiStats.toJson fails without options argument (#8561)
  • Stats assetsByChunkName is different to v4
  • node.global: false causes many errors
  • define is missing require requirement for module
  • reasons for scope-hoisted modules are inconsistent
  • resolve.alias false doesn't work
  • Source map caching is very slow
@chenxsan

This comment has been minimized.

Copy link
Member

chenxsan commented Dec 21, 2018

mini-css-extract-plugin is broken because mainTemplate.hooks.requireEnsure is removed from webpack 5, Wondering what's your plan with mini-css-extract-plugin.

@sokra

This comment has been minimized.

Copy link
Member

sokra commented Dec 21, 2018

Wondering what's your plan with mini-css-extract-plugin.

We will update mini-css-extract-plugin. There are a few new features in webpack 5 that can be now be used by the mini-css-extract-plugin: size info for styles, HMR, RuntimeModule for the css loading logic.

For now you can try with style-loader instead.

@apostolos

This comment has been minimized.

Copy link

apostolos commented Dec 21, 2018

@sokra Is cache-loader not needed anymore? Build still works but I see no performance difference. It is actually a bit slower with cache-loader.

Overall, we are noticing improved build speeds just by enabling the new cache. No problems besides the Node stuff. 馃憤

@sokra

This comment has been minimized.

Copy link
Member

sokra commented Dec 21, 2018

Is cache-loader not needed anymore?

When using the Persistent Cache, you don't need the cache-loader anymore. Same for babel cacheDirectory.

@johnnyreilly

This comment has been minimized.

Copy link
Contributor

johnnyreilly commented Dec 21, 2018

Great work @sokra! Is there an upgrade guide for plugin / loader authors available? The webpack 5 equivalent of

https://medium.com/webpack/webpack-4-migration-guide-for-plugins-loaders-20a79b927202

Or https://blog.johnnyreilly.com/2018/01/finding-webpack-4-use-map.html

?

@johnnyreilly johnnyreilly referenced this issue Dec 21, 2018

Merged

[WIP] webpack@5 `hooks` support #166

10 of 10 tasks complete
@sokra

This comment has been minimized.

Copy link
Member

sokra commented Dec 21, 2018

Is there an upgrade guide for plugin / loader authors available?

There is a section to this in the bottom half of the changelog, but it could probably need some more love.

@elliottcrush

This comment has been minimized.

Copy link

elliottcrush commented Dec 21, 2018

Apologies have re-read the changelog. Great work everyone 馃憤

@sokra

This comment has been minimized.

Copy link
Member

sokra commented Dec 21, 2018

do webpackChunkName work the same way?

yes

@gpoitch

This comment has been minimized.

Copy link
Member

gpoitch commented Dec 21, 2018

Feedback:

  1. IgnorePlugin:
    Original: new webpack.IgnorePlugin(regex)
    Build error: ValidationError: IgnorePlugin Invalid Options options should match exactly one schema in oneOf
    Solution: new webpack.IgnorePlugin({ resourceRegExp: regex })

  2. NamedChunksPlugin
    Original: new webpack.NamedChunksPlugin()
    Build error: webpack.NamedChunksPlugin is not a constructor
    Solution: optimization: { chunkIds: 'named' }

  3. mini-css-extract-plugin
    Error: Cannot read property 'tap' of undefined
    Solution: (remove plugin for now)

  4. circular-dependency-plugin
    Error: module property was removed from Dependency (use compilation.moduleGraph.getModule(dependency) instead)
    Solution: (remove plugin for now)

  5. Runtime error: ReferenceError: Can't find variable: global (jss dependency)
    Solution: new webpack.ProvidePlugin({ global: require.resolve('./global') })
    global.js: module.exports = window

@sokra

This comment has been minimized.

Copy link
Member

sokra commented Dec 21, 2018

1, 2, 5: added notes to the changelog

regarding 5: you could also set node.global: true.

@Manuelandro

This comment has been minimized.

Copy link

Manuelandro commented Dec 21, 2018

Is there something i'm missing about webpack-cli? cause they won't work together

describe: optionsSchema.definitions.output.properties.path.description,
TypeError: Cannot read property 'properties' of undefined

EDIT
Tried to remove webpack-cli and installing it based on the "recommended choices", but got a

Info All dependencies
鈹溾攢 cliui@4.1.0
鈹溾攢 global-modules-path@2.3.1
鈹溾攢 import-local@2.0.0
鈹溾攢 invert-kv@2.0.0
鈹溾攢 lcid@2.0.0
鈹溾攢 map-age-cleaner@0.1.3
鈹溾攢 mem@4.0.0
鈹溾攢 os-locale@3.0.1
鈹溾攢 p-defer@1.0.0
鈹溾攢 p-is-promise@1.1.0
鈹溾攢 resolve-cwd@2.0.0
鈹溾攢 v8-compile-cache@2.0.2
鈹溾攢 webpack-cli@3.1.2
鈹溾攢 which-module@2.0.0
鈹溾攢 yargs-parser@11.1.1
鈹斺攢 yargs@12.0.5
TypeError: statsPresetToOptions is not a function
at processOptions (/Users/m/Angular/frontend2/node_modules/webpack-cli/bin/cli.js:298:21)

@phaistonian

This comment has been minimized.

Copy link

phaistonian commented Dec 22, 2018

I presume 5.0 will essentially make hard-source-webpack-plugin obsolete.

If that's the case, is there a comparison in terms of performance?

@quetzalsly

This comment has been minimized.

Copy link

quetzalsly commented Dec 22, 2018

I can't even test this, as soon as I upgraded I just get the following error:

webpack --progress --profile --bail --config ./conf/global/webpack/prod.config
.js --profile --json > webpack-build-log.json

E:\test\test1\node_modules\webpack-cli\bin\cli.js:298
                                outputOptions = statsPresetToOptions(outputOptio
ns);

TypeError: statsPresetToOptions is not a function
@sokra

This comment has been minimized.

Copy link
Member

sokra commented Dec 22, 2018

Regarding webpack-cli: Try to pass stats: { preset: "verbose" } instead of stats: "verbose" until this is fixed (similar for other string or boolean arguments).

EDIT: Seems like the schema doesn't allow that, will fix it

EDIT: Try to omit the stats option or don't use a preset.

@evenstensberg

This comment has been minimized.

Copy link
Member

evenstensberg commented Dec 22, 2018

webpack-cli misses Stats.presetToOptions (should use compilation.createStatsOptions) any feedback on this? Also, send an issue to webpack-cli.

For v5 can we have stats.assets <Array> have the paths to their original source? It would be nice to compare % metrics with performance

@ludovicofischer

This comment has been minimized.

Copy link

ludovicofischer commented Dec 22, 2018

Crash after editing a JS file while webpack-dev-server watches for changes:

const timestamp = fileTimestamps.get(fileDependency);
TypeError: Cannot read property 'get' of undefined
    at childCompiler.fileDependencies.some (/node_modules/html-webpack-plugin/lib/compiler.js:341:38)

webpack-dev-server: 3.1.12 and html-webpack-plugin: 4.0.0-beta.5

@johnnyreilly

This comment has been minimized.

Copy link
Contributor

johnnyreilly commented Dec 23, 2018

Those looking to track the fork-ts-checker-webpack-plugin compatibility should follow the excellent @liangchunn's PR: Realytics/fork-ts-checker-webpack-plugin#166

We're at the point of having working code. We're likely to make some internal refinements and then push out an alpha. Thanks @sokra for providing advice!

The super enthusiastic can take the WIP branch for a tour here: https://github.com/liangchunn/fork-ts-checker-webpack-plugin/tree/feature/webpack5

@johnnyreilly

This comment has been minimized.

Copy link
Contributor

johnnyreilly commented Dec 23, 2018

@sokra - could you provide some details of Compilation.fileSystemInfo please? I've been digging around but I can't find any information about it. I want to switch ts-loader from using fileTimestamps to fileSystemInfo but need more information about the API of fileSystemInfo in order that I can do that.

Thanks!

@sokra

This comment has been minimized.

Copy link
Member

sokra commented Dec 23, 2018

@johnnyreilly You can call compilation.fileSystemInfo.getFileTimestamp(path, callback) to get the timestamp info of a file. It's async compared to the old API. It gives an object with { safeTime: number, timestamp: number }. safeTime is something you can compare with Date.now() (it includes an additional margin for fs accuracy). timestamp is something you store and compare with itself (you can't trust it because of fs accuracy). In future there will be a getFileHash(path, callback) methods which gives you a hash of the file content, but this is not implemented yet.

@johnnyreilly

This comment has been minimized.

Copy link
Contributor

johnnyreilly commented Dec 23, 2018

Thanks @sokra!

As it turns out, I think ts-loader may be good as is. It uses Compiler.fileTimestamps and not Compilation.fileTimestamps. Easy mistake to make 馃槃

However I think the info you've provided will be super useful for @jantimon when he's looking at html-webpack-plugin 馃憤

@capaj

This comment has been minimized.

Copy link

capaj commented Dec 23, 2018

I tried upgrading from 4.28.1 in our main app and got this:

> looop@ fe /home/capaj/git_projects/looop/project-alpha
> cd ./front-end && node dev-server

NODE_ENV:  development
/home/capaj/git_projects/looop/project-alpha/node_modules/html-webpack-plugin/index.js:59
        compilation.hooks.htmlWebpackPluginAlterChunks = new SyncWaterfallHook(['chunks', 'objectWithPluginRef']);
                                                       ^

TypeError: Cannot add property htmlWebpackPluginAlterChunks, object is not extensible
    at compiler.hooks.compilation.tap.compilation (/home/capaj/git_projects/looop/project-alpha/node_modules/html-webpack-plugin/index.js:59:56)
    at SyncHook.eval [as call] (eval at create (/home/capaj/git_projects/looop/project-alpha/node_modules/tapable/lib/HookCodeFactory.js:19:10), <anonymous>:9:1)
    at SyncHook.lazyCompileHook (/home/capaj/git_projects/looop/project-alpha/node_modules/tapable/lib/Hook.js:154:20)
    at Compiler.newCompilation (/home/capaj/git_projects/looop/project-alpha/node_modules/webpack/lib/Compiler.js:527:26)
    at hooks.beforeCompile.callAsync.err (/home/capaj/git_projects/looop/project-alpha/node_modules/webpack/lib/Compiler.js:567:29)
    at AsyncSeriesHook.eval [as callAsync] (eval at create (/home/capaj/git_projects/looop/project-alpha/node_modules/tapable/lib/HookCodeFactory.js:32:10), <anonymous>:6:1)
    at AsyncSeriesHook.lazyCompileHook (/home/capaj/git_projects/looop/project-alpha/node_modules/tapable/lib/Hook.js:154:20)
    at Compiler.compile (/home/capaj/git_projects/looop/project-alpha/node_modules/webpack/lib/Compiler.js:562:28)
    at compiler.hooks.watchRun.callAsync.err (/home/capaj/git_projects/looop/project-alpha/node_modules/webpack/lib/Watching.js:107:19)
    at AsyncSeriesHook.eval [as callAsync] (eval at create (/home/capaj/git_projects/looop/project-alpha/node_modules/tapable/lib/HookCodeFactory.js:32:10), <anonymous>:15:1)
    at AsyncSeriesHook.lazyCompileHook (/home/capaj/git_projects/looop/project-alpha/node_modules/tapable/lib/Hook.js:154:20)
    at compiler.cache.endIdle.err (/home/capaj/git_projects/looop/project-alpha/node_modules/webpack/lib/Watching.js:70:33)
    at AsyncParallelHook.eval [as callAsync] (eval at create (/home/capaj/git_projects/looop/project-alpha/node_modules/tapable/lib/HookCodeFactory.js:32:10), <anonymous>:6:1)
    at AsyncParallelHook.lazyCompileHook (/home/capaj/git_projects/looop/project-alpha/node_modules/tapable/lib/Hook.js:154:20)
    at Cache.endIdle (/home/capaj/git_projects/looop/project-alpha/node_modules/webpack/lib/Cache.js:39:22)
    at Watching._go (/home/capaj/git_projects/looop/project-alpha/node_modules/webpack/lib/Watching.js:68:23)

any pointers?

@niieani

This comment has been minimized.

Copy link
Contributor

niieani commented Dec 23, 2018

@capaj you need to upgrade html-webpack-plugin. Do not use the v4 alpha, that's outdated.

@johnnyreilly

This comment has been minimized.

Copy link
Contributor

johnnyreilly commented Dec 23, 2018

fork-ts-checker-webpack-plugin v1.0.0-alpha.0 is available now - adds support for webpack 5. Thanks for all the hard work @liangchunn!

https://github.com/Realytics/fork-ts-checker-webpack-plugin/releases/tag/v1.0.0-alpha.0

@capaj

This comment has been minimized.

Copy link

capaj commented Dec 23, 2018

@niieani thanks, that worked-but HMR is still broken, but I guess that will be problem on the side of webpack-html-plugin

@jantimon

This comment has been minimized.

Copy link
Contributor

jantimon commented Dec 23, 2018

Crash after editing a JS file while webpack-dev-server watches for changes:

const timestamp = fileTimestamps.get(fileDependency);
TypeError: Cannot read property 'get' of undefined
    at childCompiler.fileDependencies.some (/node_modules/html-webpack-plugin/lib/compiler.js:341:38)

webpack-dev-server: 3.1.12 and html-webpack-plugin: 4.0.0-beta.5

Can anyone please give me some hints how and why the timestamps api changed?

I am using it here to find out wether the html template has to be recompiled.

For webpack 4 it just loops over all compilation file dependency timestamps.
Is getFileTimestamp as fast as that look up or will it add an additional fs lookup?

@SamVerschueren

This comment has been minimized.

Copy link

SamVerschueren commented Jan 7, 2019

I agree with @sindresorhus on this one. People will start bugging package authors because things aren't working properly for them.

Look at how many times we had to link this answer sindresorhus/ama#446 why we don't want to add ES5 compatibel bundles to our modules because it didn't work for them. This is not really Webpack related, more create-react-app and angular-cli, but it's the same thing. Users expect things to work out-of-the-box.

Why not add a simple opt-out flag and document it in a performance enhancements part of the Webpack documentation?

@MLoughry

This comment has been minimized.

Copy link
Contributor

MLoughry commented Jan 7, 2019

we got a bug on global migrating to wp5 entria/entria-fullstack#46 (comment)

@MLoughry is this related to your comment above?

@sibelius, yes. However, as a co-worker later pointed out, @sokra mentioned a fix earlier in the thread :

regarding 5: you could also set node.global: true.

@sokra

This comment has been minimized.

Copy link
Member

sokra commented Jan 8, 2019

@MLoughry

This comment has been minimized.

Copy link
Contributor

MLoughry commented Jan 8, 2019

From working with @TheLarkInn, it seems that manual changes to package.json files in node_modules (to, for example, add a module field) don't invalidate the cache.

@MLoughry

This comment has been minimized.

Copy link
Contributor

MLoughry commented Jan 8, 2019

So far, I've been utterly unable to create a minimal repro; however, when importing mark.js, which has a crazy legacy UMD wrapper, the module throws an exception because module is not defined.

Debugging between the repro on my large codebase, and my attempt at a minimal repro that doesn't repro the problem, it seems that parser hook for module.exports in NodeStuffPlugin fires for the latter, but not the former (despite being installed in both cases)

@MLoughry

This comment has been minimized.

Copy link
Contributor

MLoughry commented Jan 8, 2019

Additionally, running into issues shimming the Node stream package with stream-browserify. The latter depends on readable-stream@^2.0.2, which uses the process-nexttick-args package, which references process, which the browser throws on.

browserify/stream-browserify#18 is open to upgrade to readable-stream@^3.0.0, but there's been no traction in two months.

@MLoughry

This comment has been minimized.

Copy link
Contributor

MLoughry commented Jan 8, 2019

It looks like System.import() is deprecated (or, at least, my code is throwing that System is undefined), but this is not documented in the Changelog.

Additionally, if this is an intended change, this will cause issues during Typescript transpilation when a Typescript module tries to dynamically import a non-TS/JS file.

@PlayMa256

This comment has been minimized.

Copy link
Member

PlayMa256 commented Jan 8, 2019

System.import was deprecated a long ago. Now just import

@MLoughry

This comment has been minimized.

Copy link
Contributor

MLoughry commented Jan 8, 2019

System.import was deprecated a long ago. Now just import

The problem with that is if you have Typescript code like so:

import('./someStyles.css');

then tsc will throw an error:

index.ts:15:51 - error TS2307: Cannot find module './someStyles.css'.

@MLoughry

This comment has been minimized.

Copy link
Contributor

MLoughry commented Jan 9, 2019

I've opened Microsoft/TypeScript#29315 to see if they're willing to make a change on their side.

@glen-84

This comment has been minimized.

Copy link

glen-84 commented Jan 9, 2019

@MLoughry,

It looks like System.import() is deprecated (or, at least, my code is throwing that System is undefined), but this is not documented in the Changelog.

I think it's this:

SystemPlugin is now disabled by default.

  • MIGRATION: Avoid using it as the spec has been removed. You can re-enable it with Rule.parser.system: true

Additionally, if this is an intended change, this will cause issues during Typescript transpilation when a Typescript module tries to dynamically import a non-TS/JS file.

You should be able to add this to a .d.ts file:

declare module "*.css";

@sokra sokra pinned this issue Jan 9, 2019

@sokra

This comment has been minimized.

Copy link
Member

sokra commented Jan 9, 2019

@realityking

This comment has been minimized.

Copy link

realityking commented Jan 9, 2019

Regarding the node-browser-libs discussion - I'll be very happy when the dependency disappears. It not only makes webpack much bigger than it needs to be, it also gives a false sense of safety - many modules are unmaintained and not in sync with the Node.js code.

I understand this will cause some developers to open tickets asking for browser support, or worse, asking why the library doesn't work with webpack.

To give a sane default but not cause too much confusion, how about
a) providing a webpack plugin that automatically loads dependencies included node-libs-browser and
b) adding a hint to the error message "Module not found" pointing to this plug-in. Bonus points for showing the hint only when it's one of the built-in modules.

@smikula

This comment has been minimized.

Copy link

smikula commented Jan 9, 2019

In the case of scope-hoisted modules, when examining the individual modules, the reasons are missing for the primary/root module. In the example below two modules (X.js and Y.js) have been merged into X.js + 1 modules. Notice that the reasons for X.js is just an empty array in the alpha build. (Is this intentional? Arguably those reasons are redundant with the reasons for the top level X.js + 1 modules module.)

Here's the module structure (with irrelevant fields removed) from stats generated by 5.0.0-alpha.5:

{
  "name": "./lib/X.js + 1 modules",
  "modules": [
    {
      "name": "./lib/Y.js",
      "reasons": ["   /* These reasons look fine */   "]
    },
    {
      "name": "./lib/X.js",
      "reasons": []
    }
  ]
}

From stats generated by 4.28.1:

{
  "name": "./lib/X.js + 1 modules",
  "modules": [
    {
      "name": "./lib/X.js",
      "reasons": [
        {
          "module": "./lib/A.js + 3 modules",
          "type": "harmony side effect evaluation"
        },
        {
          "module": "./lib/A.js + 3 modules",
          "type": "harmony export imported specifier"
        }
      ],
    },
    {
      "name": "./lib/Y.js",
      "reasons": ["   /* These reasons look fine */   "],
    }
  ]
}
@MLoughry

This comment has been minimized.

Copy link
Contributor

MLoughry commented Jan 9, 2019

So far, I've been utterly unable to create a minimal repro; however, when importing mark.js, which has a crazy legacy UMD wrapper, the module throws an exception because module is not defined.

So, turns out I was using mark.js@8.11.1 in my attempted repro, which doesn't repro the issue.

Regardless, using mark.js@8.11.0 works in webpack 4, but causes a runtime exception in webpack 5. In the JavascriptParser, it evaluates the expression typeof define === "function" && define.amd as always true, but then at runtime it throws because no module identifier is exposed for this block of emitted code:

(function (factory, window, document) {
    if (true) {
        !(__WEBPACK_AMD_DEFINE_ARRAY__ = [], __WEBPACK_AMD_DEFINE_RESULT__ = (function () {
            return factory(window, document);
        }).apply(exports, __WEBPACK_AMD_DEFINE_ARRAY__),
		__WEBPACK_AMD_DEFINE_RESULT__ !== undefined && (module.exports = __WEBPACK_AMD_DEFINE_RESULT__));
    } else {}
})
@sokra

This comment has been minimized.

Copy link
Member

sokra commented Jan 10, 2019

b) adding a hint to the error message "Module not found" pointing to this plug-in. Bonus points for showing the hint only when it's one of the built-in modules.

We have a hint that shows the necessary configuration to include a polyfill manually:

ERROR in ./index.js 2:0-13
Module not found: Error: Can't resolve 'os' in 'Xdir/module-not-found-error'
BREAKING CHANGE: webpack < 5 used to include polyfills for node.js core modules by default.
This is no longer the case. Verify if you need these module and configure a polyfill for it.
If you want to include a polyfill, you need to:
- add an alias 'resolve.alias: { \\"os\\": \\"os-browserify/browser\\" }'
- install 'os-browserify'
If you don't want to include a polyfill, you can use an empty module like this:
resolve.alias: { \\"os\\": false }

The intend was to make the developer aware of the included polyfill and they should be able to decide if it makes sense to include it. It may could be further improved by adding the size of the polyfill to the message.

@phaistonian

This comment has been minimized.

Copy link

phaistonian commented Jan 10, 2019

@SevInf Run those tests again with new alpha?

@errorx666

This comment has been minimized.

Copy link

errorx666 commented Jan 10, 2019

@sokra Note that it suggests

resolve.alias: { \\"os\\": false } 

but this is actually rejected by the schema.

- configuration.resolve.alias should be one of these:
   object { <key>: string } | [object { alias?, name?, onlyModule? }]
   -> Redirect module requests
   Details:
    * configuration.resolve.alias['os'] should be a string.
      -> New request
    * configuration.resolve.alias should be an array:
      [object { alias?, name?, onlyModule? }]
@MLoughry

This comment has been minimized.

Copy link
Contributor

MLoughry commented Jan 10, 2019

When building multiple entrypoints with bail: true, webpack crashes with the following error:

Error: Queue was stopped
    at AsyncQueue.add (E:\VSO\webpack-5-issues\node_modules\webpack\lib\util\AsyncQueue.js:65:38)
    at Compilation.buildModule (E:\VSO\webpack-5-issues\node_modules\webpack\lib\Compilation.js:685:19)
    at addModule (E:\VSO\webpack-5-issues\node_modules\webpack\lib\Compilation.js:931:11)
    at hooks.result.callAsync.hookError (E:\VSO\webpack-5-issues\node_modules\webpack\lib\util\AsyncQueue.js:232:5)
    at SyncHook.eval [as callAsync] (eval at create (E:\VSO\webpack-5-issues\node_modules\tapable\lib\HookCodeFactory.js:32:10), <anonymous>:6:1)
    at AsyncQueue._handleResult (E:\VSO\webpack-5-issues\node_modules\webpack\lib\util\AsyncQueue.js:216:21)
    at process.nextTick (E:\VSO\webpack-5-issues\node_modules\webpack\lib\util\AsyncQueue.js:199:12)
    at process.internalTickCallback (internal/process/next_tick.js:70:11)

It seems that the callback in Compilation.addModuleChain stops all async queues in its callback if bail is true, regardless of whether or not there's an error.

@MLoughry

This comment has been minimized.

Copy link
Contributor

MLoughry commented Jan 10, 2019

When building a sufficiently large codebase, the code to emit the cache file can crash.

E:\vso\client-web\node_modules\webpack\lib\serialization\FileMiddleware.js:307
                            throw err;
                            ^

RangeError [ERR_INVALID_OPT_VALUE]: The value "2629006079" is invalid for option "size"
    at Function.allocUnsafe (buffer.js:284:3)
    at Function.concat (buffer.js:466:23)
    at Object.fileManager.addJob [as fn] (E:\vso\client-web\node_modules\webpack\lib\serialization\FileMiddleware.js:300:55)
    at process (E:\vso\client-web\node_modules\webpack\lib\serialization\FileMiddleware.js:189:24)
    at fs.open (E:\vso\client-web\node_modules\webpack\lib\serialization\FileMiddleware.js:178:21)
    at FSReqCallback.args [as oncomplete] (fs.js:147:20)

@jpavon jpavon referenced this issue Jan 10, 2019

Open

Webpack 5 #9

@SevInf

This comment has been minimized.

Copy link

SevInf commented Jan 11, 2019

I redid my tests on the latest alpha. Memory consumtion improved a little, but I see no difference in build times and #8591 still brings significant improvements. Here are few sample results:

alpha 5, clean build
        User time (seconds): 1263.31
        System time (seconds): 197.69
        Percent of CPU this job got: 270%
        Elapsed (wall clock) time (h:mm:ss or m:ss): 8:59.74
        Maximum resident set size (kbytes): 4353748
alpha 5, incremental build with no changes
        User time (seconds): 69.32
        System time (seconds): 12.29
        Percent of CPU this job got: 124%
        Elapsed (wall clock) time (h:mm:ss or m:ss): 1:05.47
        Maximum resident set size (kbytes): 2773048
alpha 5 + #8591, clean build
        User time (seconds): 1150.69
        System time (seconds): 202.82
        Percent of CPU this job got: 318%
        Elapsed (wall clock) time (h:mm:ss or m:ss): 7:04.66
        Maximum resident set size (kbytes): 4279088

Unfortunately, I could not get the CPU profile for the whole build (Chrome Dev Tools crashes), but here is a profile of a build, reduced to 52 entrypoints:
reduced.cpuprofile.zip

@hulkish

This comment has been minimized.

Copy link
Contributor

hulkish commented Jan 13, 2019

When building multiple entrypoints with bail: true, webpack crashes with the following error:

@MLoughry This fixes that. Just waiting on review

@sokra

This comment has been minimized.

Copy link
Member

sokra commented Jan 15, 2019

@MLoughry I guess #8622 fixes the large cache crash.

2.5GB cache file... Maybe try to switch to store: "idle" which stores each cache entry in a separate file, so you only have to rewrite these files.

@sokra

This comment has been minimized.

Copy link
Member

sokra commented Jan 15, 2019

https://github.com/webpack/webpack/releases/tag/v5.0.0-alpha.6

@SevInf This still don't include #8591, but I didn't forget it. Want to move serializer into separate package and move that logic into webpack-sources.

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