Skip to content
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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add output.futureEmitAssets #8642

Merged
merged 2 commits into from Jan 19, 2019
Merged

add output.futureEmitAssets #8642

merged 2 commits into from Jan 19, 2019

Conversation

sokra
Copy link
Member

@sokra sokra commented Jan 17, 2019

add a new version of emitting assets which allows to free memory of Sources with the trade-off of disallowing reading asset content after emitting

It also uses Source.buffer when available.

What kind of change does this PR introduce?
memory

Did you add tests for your changes?
no (it will be tested in the next branch, when it's on by default)

Does this PR introduce a breaking change?
no

What needs to be documented once your changes are merged?
New option output.futureEmitAssets allows to opt-in to the new way of emitting assets which allows to free memory of Sources with the trade-off of disallowing reading asset content after emitting

add a new version of emitting assets which allows to free memory of Sources with the trade-off of disallowing reading asset content after emitting

It also uses Source.buffer when available.
@webpack-bot
Copy link
Contributor

For maintainers only:

  • This need to be documented (issue in webpack/webpack.js.org will be filed when merged)

@webpack-bot
Copy link
Contributor

Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon.

Copy link
Contributor

@dav-is dav-is left a comment

Choose a reason for hiding this comment

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

Fixes the memory issue on our end

@dav-is
Copy link
Contributor

dav-is commented Jan 17, 2019

When using the flag on a development build, it freezes on WebpackDevMiddleware. It doesn't appear to throw any errors. It accesses source() after emit https://github.com/webpack/webpack-dev-middleware/blob/90d0d94c76e51c3b55cdd2b19978c8cd78edcf85/lib/fs.js#L28

@sokra
Copy link
Member Author

sokra commented Jan 19, 2019

You don't have to use writeToDisk

@sokra sokra merged commit 6389e41 into master Jan 19, 2019
@sokra sokra deleted the memory/future-emit-assets branch January 19, 2019 07:58
filipesilva added a commit to filipesilva/angular-cli that referenced this pull request Jan 22, 2019
filipesilva added a commit to filipesilva/angular-cli that referenced this pull request Jan 22, 2019
mgechev pushed a commit to angular/angular-cli that referenced this pull request Jan 22, 2019
redbugz added a commit to fs-webdev/create-react-app that referenced this pull request Apr 27, 2019
CRA 3 opts-in to a new webpack 5 feature early to save memory: `futureEmitAssets`
facebook#6696
This optimization only stores the file sizes during webpack compilation, no longer storing the original source
webpack/webpack#8642
This is incompatible with our webpack-dev-server customization setting of `writeToDisk`
webpack/webpack#8642 (comment)
which tries to read the source after emit, which throws an exception, but that error is swallowed and the `npm start` just hangs.
Before we update to webpack 5, where this feature will be the default, we need to stop writing to disk, or find another way to do it than the `writeToDisk` flag.
ikjelle pushed a commit to ikjelle/angular-cli that referenced this pull request Mar 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants