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

Limit number of files processed asynchronously #3166

Closed
wants to merge 1 commit into from

Conversation

McPo
Copy link

@McPo McPo commented Oct 20, 2016

Please check if the PR fulfills these requirements

  • [] Tests for the changes have been added (for bug fixes / features)
  • [] Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior? (You can also link to an open issue here)
OS file limit is reached by processing too many files at the same time.

What is the new behavior?
Limits the number of files to 1000 or set by an environment variable. (Maybe it should be even lower ~500)

Does this PR introduce a breaking change?

  • Yes
  • No

Other information:
Resolves #3164

@@ -232,7 +232,8 @@ Compiler.prototype.emitAssets = function(compilation, callback) {
function emitFiles(err) {
if(err) return callback(err);

require("async").forEach(Object.keys(compilation.assets), function(file, callback) {
var async = require("async");
async.forEachLimit(Object.keys(compilation.assets), process.env.WEBPACK_FILE_LIMIT || 1000, async.ensureAsync(function(file, callback) {
Copy link
Member

Choose a reason for hiding this comment

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

Don't use environment variables, instead use a configuration value.

@@ -232,7 +232,8 @@ Compiler.prototype.emitAssets = function(compilation, callback) {
function emitFiles(err) {
if(err) return callback(err);

require("async").forEach(Object.keys(compilation.assets), function(file, callback) {
var async = require("async");
Copy link
Member

Choose a reason for hiding this comment

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

async will become a reversed keyword, rename it.

Copy link
Member

Choose a reason for hiding this comment

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

@sokra what name should be preferred for the var? asyncLib? Might be worth it to make a PR rewriting all uses of async as an identifier.

Copy link
Member

Choose a reason for hiding this comment

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

yep, asyncLink sounds fine

@sokra
Copy link
Member

sokra commented Dec 7, 2016

This PR should be done against the master branch not only webpack 1

Copy link
Member

@sokra sokra left a comment

Choose a reason for hiding this comment

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

rebase against master

@sokra
Copy link
Member

sokra commented Aug 11, 2017

see #5502

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