NXT-18691: Added improvements for webpack - #165
Open
daniel-stoian-lgp wants to merge 1 commit into
Open
Conversation
daniel-stoian-lgp
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist
Issue Resolved / Feature Added
mixins/unmangled.js assigned terserPlugin.options.terserOptions. But terser-webpack-plugin destructures terserOptions in its constructor, resolves it into options.minimizer.options, and reads only that at runtime — options.terserOptions does not exist on the instance afterwards.
Every --no-minify build therefore produced fully-minified, fully-mangled output. The only effect that did apply was config.output.pathinfo, which is why dist/ totals looked almost right (60.0 MB vs 59.7 MB) rather than obviously broken — the flag looked like it was working.
Verified directly:
has .options.terserOptions after construction? false
mangle after the old-style mutation: {"safari10":true} <-- unchanged
The fix mutates the resolved options object instead.
Command Before After
pack -p --no-minify — main.js 1114 KB (identical to a minified build) 3695 KB
handleManifestFiles read one file at a time via callback-per-file recursion (fs.readFile → wait → recurse). Across the ~6,750-file iLib tree that serialises thousands of disk round-trips into the build's critical path.
There is also a related issue under serve: shouldEmit decides whether to skip a file by stat-ing its destination, but the dev server's output filesystem is in-memory (webpack-dev-middleware), so there is never a destination to stat — every one of the ~6,750 files was re-read from disk on every single compile of a dev-server session, not just the first.
This change fans reads out over a bounded (32-way) concurrency pool and adds a source-keyed (mtime + size) read cache so repeat compiles in the same process reuse the already-read buffer. shouldEmit now returns its already-computed source fs.Stats alongside the decision, so each file is stat-ed exactly once — same as before, not twice.
Resolution
Additional Considerations
Links
NXT-18691
Comments