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

"RangeError: Maximum call stack size exceeded" when compiling a file with many requires #771

Closed
Gobie opened this issue Sep 17, 2018 · 18 comments · Fixed by #1525
Closed

Comments

@Gobie
Copy link

Gobie commented Sep 17, 2018

Created based on comment #689 (comment)

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
Given CSS file with 10000 or more classes with URLs, in our case generated from sprites.

...
.classN { ... background-image: url(./path/to/sprite.png); ... }
...

The css-loader creates such a JS file, which on compile throws RangeError: Maximum call stack size exceeded.

...
exports.push([
  module.id, 
  ".class0 { background-image: url(" +
  escape(require("./path/to/sprite.png")) +
  "); }.class1 { background-image: url(" +
  escape(require("./path/to/sprite.png")) +
  ...
])

What is the expected behavior?
The expected behaviour is to generate a JS file, which can be compiled and executed.

Please mention other relevant information such as your webpack version, Node.js version and Operating System.
Reproducible on 1.0.0, Node 6.11.3 & Node 8.9.4.

Technical details
The issue is that URLs are replaced by requires and cssAsString in loader.js is built by string concatenation "..." + "..." + ... + "..." which has upper limit in each version of v8, given by it's default stack size.

Experimentally measured:

  • node 6.11.3 has ~6250 strings,
  • node 8.9.4 has ~3285 strings.

The fix is straightforward, group the string concatenations by fixed amount so it can be compiled using default stack size. ("..." + ... + "...") + ("..." + ... + "...")

Smallest possible test-case for css-loader

/*globals describe */

var helpers = require("./helpers");
var test = helpers.test;

describe("string concat", function() {
	this.timeout(20000);

	var actualCSS, expectedCSS, i;

	actualCSS = '';
	expectedCSS = '';
	for (i = 0; i < 10000; i++) {
		actualCSS += ".class" + i + " { background-image: url(./path/to/file.png); }";
		expectedCSS += ".class" + i + " { background-image: url({./path/to/file.png}); }";
	}

	test("should handle concat of 20001 strings", actualCSS, [[1, expectedCSS, ""]]);
});

Repo with reproducible test-case https://github.com/Gobie/css-loader/tree/issue-771

@alexander-akait
Copy link
Member

@Gobie Thanks for issue

@Gobie Gobie changed the title RangeError: Maximum call stack size exceeded" when compiling a file with many requires "RangeError: Maximum call stack size exceeded" when compiling a file with many requires Oct 10, 2018
@alexander-akait alexander-akait added this to the 2.0.1 milestone Oct 30, 2018
@alexander-akait alexander-akait modified the milestones: 2.0.1, 2.0.0 Dec 5, 2018
@alexander-akait
Copy link
Member

Using ("..." + ... + "...") + ("..." + ... + "...") is not help because webpack use acorn and some plugins for acorn have same problem, don't know how better fix it, feel free to send a PR if you have ideas

@alexander-akait
Copy link
Member

Can't be solved on css-loader side, need update acorn in webpack (in webpack@5 it is already done). Thanks for issue.

@sgarfinkel
Copy link

Hi @alexander-akait, we are encountering this too, due to the same underlying issue (too many string concatenations because we have so many URLs). I think there are two solutions: Use a template literal to interpolate the resolved URLs, or use [].join(''). I haven’t tested this, but presumably it would workaround the limit. Would this be something we could implement?

@sgarfinkel
Copy link

The other option is to just call this.loadModule to resolve each url in the loader itself then we wouldn’t need webpack to evaluate at all at compile time (and could just pass the string in directly).

@alexander-akait
Copy link
Member

Honestly, could you count the number of them?

The other option is to just call this.loadModule to resolve each url in the loader itself then we wouldn’t need webpack to evaluate at all at compile time (and could just pass the string in directly).

Will be very bad for perfromance, because if you will have mini-css-extract-plugin, you will do it twicy, but evalation is always slow

@alexander-akait
Copy link
Member

We can add ( and ) to prevent it, but I need to verify you really faced with it

@sgarfinkel
Copy link

Let me try to create a minimal reproduction for you. It looks like the one in the original ticket is gone.

@alexander-akait
Copy link
Member

@sgarfinkel Feel free to ping me when it will be ready

@alexander-akait
Copy link
Member

@sgarfinkel Reproduce it, with 100_000 URLs 😄

@alexander-akait
Copy link
Member

The original problem was solved and our limits are more than 6k, the code was reworked, but yeah, using very many string concatination (binary expressions) are still broken

@sgarfinkel
Copy link

sgarfinkel commented May 27, 2023

I reproduced it with 10_000 concatenations. I don’t know what the limit is, but in my use case I’m loading many sass style sheets, possibly multiple times. I minify at the end to eliminate the duplicates but it causes many concatenations. I was able to solve this by switching to using templated strings instead. Would you accept that PR?

@alexander-akait
Copy link
Member

alexander-akait commented May 27, 2023

@sgarfinkel I am afraid we can't use templated strings, because it is code for runtime (browser runtime) and we still support old browsers, they just will not work

@alexander-akait
Copy link
Member

I have solution to split code into multiple parts between replacers (i.e. urls/locals/etc) using variables, but I tested the performance of this code and it drops quite a lot...

@sgarfinkel
Copy link

In that case you could use [].join()

@sgarfinkel
Copy link

The last option is to use loadModule and string templates when emitting to just emit a large string without interpolation.

@alexander-akait
Copy link
Member

The last option is to use loadModule and string templates when emitting to just emit a large string without interpolation.

Sorry, no, we can't, it decrases perfomance very very very, because on every thing we need to execute JS

@alexander-akait
Copy link
Member

@sgarfinkel Okay, let' use this #1525, shorty:

  • if your enviroment supports templateLiteral, i.e. output. environment,templateLiteral: true we will use it (if you have modern browsers it will be true for you), otherwise you can set it to true for dev enviroment, because in production mini-css-extract-plugin will extract it to file and you will not have the such problem
  • otherwise we will use binary, using ( "string" + "string" ) + ( "string" + "string" ) doesn't work, because acorn fails in other places (tokenizer), but I still think what acorn should use another approach to solve it

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

Successfully merging a pull request may close this issue.

3 participants