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

Max Stack Size Exceeded for huge HTML #4694

Closed
siebeneicher opened this issue Apr 20, 2020 · 33 comments
Closed

Max Stack Size Exceeded for huge HTML #4694

siebeneicher opened this issue Apr 20, 2020 · 33 comments
Labels
compiler Changes relating to the compiler good first issue A small, encapsulated issue, ideal for a new contributor to Svelte to tackle.

Comments

@siebeneicher
Copy link

siebeneicher commented Apr 20, 2020

Describe the bug
The content of the the svelte file is a large HTML chunk, without any script logic or styling being involved. Max stack size exceeded error while compiling.

To Reproduce
https://svelte.dev/repl/a9dfcc17551c4aeb95e8fe748a97061d?version=3.20.1

Expected behavior
Compiling should not break

Information about your Svelte project:
Svelte 3.20.1, Rollup, Windows

Severity
Its probably an uncommon case for most people. I ran into it, when using a generated data privacy HTML template. I would not see this as a priority. Workaround for me: copy the HTML into a JS template literal variable and inject it via @html: https://svelte.dev/repl/1ab32c5a3c2c426e973512cfc8da023c?version=3.20.1

@pushkine
Copy link
Contributor

the compiling process involves push spreading all nodes into another array at some point and the v8 engine does not support spreading that many items
works fine on firefox

@Conduitry
Copy link
Member

Interesting. I've run into this before in V8 and ended up using an array spread and a reassignment, rather than an argument spread in .push(). Conduitry/do-not-zip@861a929 It's not very helpful that V8 gives a stack size error rather than an argument count error.

I'm not sure how many places we're doing .push(...foo) in the codebase, but this might be a way forward.

@antony antony added the good first issue A small, encapsulated issue, ideal for a new contributor to Svelte to tackle. label Apr 24, 2020
@mrbarletta
Copy link

mrbarletta commented May 23, 2020

Same issue when using Tailwind UI.

In a regular svelte + tailwindcss project if I add in the tailwind.config.js the tailwind UI package plugins: [ require('@tailwindcss/ui'),] then the building breaks with the same issue.

Could we update the issue name as well to include tailwindUI word? @siebeneicher

@milahu
Copy link
Contributor

milahu commented Jun 1, 2020

same here

im building a large single page app / single file app, with many many html nodes

error is

[!] (plugin svelte) RangeError: Maximum call stack size exceeded
src/Main.svelte
RangeError: Maximum call stack size exceeded
    at /tmp/x/node_modules/svelte/node_modules/code-red/dist/code-red.mjs:581:10
    at /tmp/x/node_modules/svelte/node_modules/code-red/dist/code-red.mjs:226:10
    at handle (/tmp/x/node_modules/svelte/node_modules/code-red/dist/code-red.mjs:70:17)
    at /tmp/x/node_modules/svelte/node_modules/code-red/dist/code-red.mjs:248:18
    at Array.map (<anonymous>)
    at handle_body (/tmp/x/node_modules/svelte/node_modules/code-red/dist/code-red.mjs:247:21)
    at Program (/tmp/x/node_modules/svelte/node_modules/code-red/dist/code-red.mjs:318:10)
    at handle (/tmp/x/node_modules/svelte/node_modules/code-red/dist/code-red.mjs:70:17)
    at print (/tmp/x/node_modules/svelte/node_modules/code-red/dist/code-red.mjs:1413:17)
    at Component.generate (/tmp/x/node_modules/svelte/src/compiler/compile/Component.ts:316:9)

with npm install code-red

node_modules/code-red/dist/code-red.mjs line 581

		chunks.push(
			c(') '),
			...handle(node.body, state)
		);

.... as expected, using push instead of reassign

compiles to node_modules/svelte/compiler.js line 5911

			const chunks = [c('for (')];
			//
			chunks.push(
				c(') '),
				...handle(node.body, state)
			);

todo: push ----> reassign

like

			let chunks = [c('for (')];
			//
			chunks = [
				...chunks,
				c(') '),
				...handle(node.body, state)
			];

@milahu
Copy link
Contributor

milahu commented Jun 2, 2020

fixed : )

here is a quick-and-dirty patch script, to replace push with reassign

patch-svelte-compiler.mjs
/*

  patch-svelte-compiler.mjs

  replace push with reassign to fix https://github.com/sveltejs/svelte/issues/4694

  1. save this script in your project folder - note the extension mjs
  2. make sure this file exists: node_modules/svelte/compiler.js
  3. run: node patch-svelte-compiler.mjs

*/

// npm install acorn estree-walker magic-string
import {parse as acorn_parse} from "acorn";
import {walk as estree_walk} from "estree-walker";
import magicString from "magic-string";
import * as fs from "fs";

// replaceMethod
//   origin: a.push(...a1, ...a2, e, ...a3); // error: Max Stack Size Exceeded
//   spread: a = [...a1, ...a2, e, ...a3];
//   concat: a = a1.concat(a2, [e], a3);
//   performance is equal on nodejs
const replaceMethod = "spread";
//const replaceMethod = "concat";

var base_file = process.argv[2] || "node_modules/svelte/compiler";

const base_file_list = [
  `${base_file}.js`, // "type": "commonjs"
  `${base_file}.mjs`, // "type": "module"
];

for (const base_file of base_file_list) {

  const backup_file = base_file + ".bak";
  const temp_file = base_file + ".tmp";

  if (fs.existsSync(backup_file)) {
    console.log(`error: backup file exists (${backup_file}). run this script only onc`);
    continue;
  }

  if (fs.existsSync(temp_file)) {
    console.log(`error: temporary file exists (${temp_file}). run this script only onc`);
    continue;
  }

  // input
  const content = fs.readFileSync(base_file, 'utf8');

  // output
  let code = new magicString(content);

  const ast = acorn_parse(
    content, {
    ecmaVersion: 2020,
    sourceType: 'module',
  });

  const funcName = "push";

  let arrayNameList = [];

  estree_walk( ast, {
    enter: function ( node, parent, prop, index ) {

      // node must be array.push()
      if (
        node.type !== 'CallExpression' ||
        node.callee === undefined ||
        node.callee.property === undefined ||
        node.callee.property.name !== funcName
      ) { return; }

      // argument list must include spread operators
      if (node.arguments.find(
        a => (a.type == 'SpreadElement')) === undefined)
      { return; }

      const nodeSrc = content.substring(node.start, node.end);

      const pushObj = node.callee.object;
      const arrayName = content.substring(pushObj.start, pushObj.end);

      const pushProp = node.callee.property;

      arrayNameList.push(arrayName);

      // patch .push(

      if (replaceMethod == "spread") {
        // push --> assign array

        // find "(" bracket after .push
        const pushPropLen = content.substring(pushProp.start, node.end).indexOf("(");

        code.overwrite(
          (pushProp.start - 1),
          (pushProp.start + pushPropLen + 1),
          " /* PATCHED */ = [..."+arrayName+", "
        );

        // patch closing bracket
        const closeIdx = node.start + nodeSrc.lastIndexOf(")");
        code.overwrite(closeIdx, (closeIdx + 1), "]");
      }

      if (replaceMethod == "concat") {
        // push --> assign concat
        // ".push" --> " = array.concat"
        code.overwrite(
          (pushProp.start - 1),
          pushProp.end,
          " /* PATCHED */ = "+arrayName+".concat");

        // patch arguments of .concat()
        node.arguments.forEach(a => {
          if (a.type == 'SpreadElement') {
            // unspread: ...array --> array
            const spreadArgSrc = content.substring(a.argument.start, a.argument.end);
            //console.log('spread argument: '+spreadArgSrc);
            code.overwrite(a.start, a.end, spreadArgSrc);

          } else {
            // enlist: element --> [element]
            const argSrc = content.substring(a.start, a.end);
            //console.log('non spread argument: '+argSrc);
            code.overwrite(a.start, a.end, "["+argSrc+"]");
          }
        });
      }

  }});

  code = code.toString();

  function filterUnique(value, index, array) { 
    return array.indexOf(value) === index;
  }

  // replace const with let
  arrayNameList.filter(filterUnique).forEach(arrayName => {
    console.log(`arrayName = ${arrayName}`)

    code = code.replace(
      new RegExp("const "+arrayName+" = ", 'g'), // global = replace all
      "/* PATCHED const "+arrayName+" */ let "+arrayName+" = "
    );
  })

  fs.writeFileSync(temp_file, code);

  console.log(`move file: ${base_file} --> ${backup_file}`)
  fs.renameSync(base_file, backup_file);

  console.log(`move file: ${temp_file} --> ${base_file}`)
  fs.renameSync(temp_file, base_file);

}

console.log(`\n\nundo:\n`);
for (const base_file of base_file_list) {
  console.log(`mv ${base_file}.bak ${base_file}`);
}

console.log(`\n\ncompare:\n`);
for (const base_file of base_file_list) {
  console.log(`git diff ${base_file}.bak ${base_file}`);
}

console.log(`\n\ncreate patch:\n`);
console.log(`npx patch-package svelte`);

edit: included patch by rhythm-section
edit: move input/output files in javascript
edit: added replaceMethod = "concat", but performance is equal
edit: moved to <details>
edit: there is a fork at https://gist.github.com/NetOperatorWibby/8515baa503ccc7b60189c6871c2c6096
edit: upgrade to ESM

@rhythm-section
Copy link

rhythm-section commented Jun 4, 2020

@milahu Thank you for the compiler patch. It works great. With version 3.23.0 of svelte I still had one issue with the patch that resulted in an error because of line 6442 inside comiler.js:

const chunks = node.kind !== 'init'
  ? [c(`${node.kind} `)]
  : [];

Because of the tertiary operator the const declaration does not get replaced with let which leads to an assignment error.

Not sure if it is a good solution or if it leads to other issues with other svelte versions, but the following worked for me. I changed the regex from:

new RegExp("const "+arrayName+" = \\[", 'g'), // global = replace all
    "/* PATCHED const "+arrayName+" */ let "+arrayName+" = ["

to:

new RegExp("const "+arrayName+" = ", 'g'), // global = replace all
    "let "+arrayName+" = "

Hope this helps someone still having troubles.

@multipliedtwice
Copy link

Is there any CI-able solution?

@milahu
Copy link
Contributor

milahu commented Sep 1, 2020

you mean patching the typescript source files?

here is a start:
https://github.com/milahu/random/blob/master/svelte/patch-svelte-compiler-sources.js

todo:

  • find source files of svelte/compiler.js via sourcemaps
  • if needed, add javascript transformer in second patch pass
  • send PRs to affected libraries

cosmetic:

  • avoid second patch pass, use node.parent to find declaration in local scope

@milahu
Copy link
Contributor

milahu commented Sep 25, 2020

the problem with concat is: concat is slower than push
= overhead of immutable data

this should be fastest:

Array.prototype._concat_inplace = function(other) { // aka _push_array
  for (let i = 0; i < other.length; i++) {
    this.push(other[i]);
  }
  return this; // chainable
};

array1._concat_inplace(array2)._concat_inplace(array3);

its even faster than array1.push(...array2)
and it is not limited by max call stack size (node ~100K, firefox ~500K)

@Spansky
Copy link

Spansky commented Nov 5, 2020

This issue is affecting working with .svg aswell. I have a bunch of huge SVGs that I want to animate with Svelte (in Sapper). Unfortunately I run into

✗ client
Maximum call stack size exceeded

My workaround is to downsize the svg, by reducing the complexity of paths used in it.

@shashank-sachan
Copy link

Is there any fix available for this? I'm also facing the max stack size issue while importing the big SVG file as svelte component.

@milahu
Copy link
Contributor

milahu commented Dec 26, 2020

there is a fix hidden 7 posts above

but it may be better to outsource static parts, at least the svg <defs>
can you tell more on your use case?

@shashank-sachan
Copy link

there is a fix hidden 7 posts above

but it may be better to outsource static parts, at least the svg <defs>
can you tell more on your use case?

@milahu My SVG is a svelte component (converted from .svg to .svelte) as I have to attach the multiple events like open a pop-up if anyone clicks on any specific area or show a image slider on screen (dynamically using the api data) inside that SVG.

@milahu
Copy link
Contributor

milahu commented Dec 27, 2020

you can get it running with my patch, but development will be slower than with smaller components

another solution: https://svelte.dev/repl/b0910aa018514053ae798250e17faa7a
= include your svg code with {@html '<svg>....</svg>'} and use onMount to attach event-listeners

@NetOpWibby
Copy link

NetOpWibby commented May 27, 2021

This should really be fixed.

Much thanks to @milahu for the excellent workaround

@davidjamesb
Copy link

Discovered this whilst evaluating SvelteKit (my issue was closed above). Trying to convert a large static HTML theme.

@NetOperatorWibby - I've attempted to use the patch here (https://gist.github.com/NetOperatorWibby/8515baa503ccc7b60189c6871c2c6096). I ran all 4 commands without error but I am still seeing the 'maximum call stack size' error. I'm not a front-end engineer at heart so I'm not sure how to proceed with this. The original patch was submitted over a year ago.

Breaking it down and composing smaller components doesn't appear to have any effect.

System:
OS: Windows 10 10.0.19041
CPU: (8) x64 Intel(R) Core(TM) i7-9700K CPU @ 3.60GHz
Memory: 14.04 GB / 31.94 GB
Binaries:
Node: 14.17.3 - C:\Program Files\nodejs\node.EXE
Yarn: 1.22.10 - ~\AppData\Roaming\npm\yarn.CMD
npm: 6.14.13 - C:\Program Files\nodejs\npm.CMD
Browsers:
Chrome: 92.0.4515.107
Edge: Spartan (44.19041.1023.0), Chromium (92.0.902.62)
Internet Explorer: 11.0.19041.1
npmPackages:
@sveltejs/kit: next => 1.0.0-next.139
svelte: ^3.34.0 => 3.41.0

@milahu
Copy link
Contributor

milahu commented Aug 2, 2021

but I am still seeing the 'maximum call stack size' error

whats the exact error trace?
what code throws the error? (in compiler.js)
see my post "same here" for more details #4694 (comment)

maybe your node uses compiler.mjs which is not patched

@davidjamesb
Copy link

Yeah, looks like compiler.mjs. My original issue with the details is here: (sveltejs/kit#2061).

image

@milahu
Copy link
Contributor

milahu commented Aug 2, 2021

looks like compiler.mjs

ive updated my patch-svelte-compiler.js in #4694 (comment)

//var base_file = "compiler.mjs"; // "type": "module" // TODO verify

uncomment that line, run the script and hope for the best ; )

@davidjamesb
Copy link

looks like compiler.mjs

ive updated my patch-svelte-compiler.js in #4694 (comment)

//var base_file = "compiler.mjs"; // "type": "module" // TODO verify

uncomment that line, run the script and hope for the best ; )

@milahu Haven't been able to get back to this for a few days. Thanks a lot - that works great and the error is no longer present.

@benbender
Copy link
Contributor

I encountered this one today. Just wanted to ask if there is a specific reason, why there seems to be a working patch but no pr to bring that fix into svelte? Several hundret html-tags on a page don't seem that unreasonable to me today and if this is really a wontfix, it should be at least documented as it could be easily a dealbreaker for specific usecases.

@milahu
Copy link
Contributor

milahu commented Sep 5, 2021

no pr to bring that fix into svelte?

this would require patching multiple packages (svelte, code-red, ...) which is ... work
ive started a script to autopatch typescript sources, but ... im too happy with my workaround (patch only the compiler.js)

@benbender
Copy link
Contributor

The crash(!) occurs with no special prerequisites in quite simple, normal usecases and without documentation or warning (see my repro https://svelte.dev/repl/0a52a75c95d34423bf6d328ca4e1db88?version=3.42.4). The problem worsens as svelte is advertised to be especially able to handle very large node-counts for visualization etc better than react & co.

Imho(!), apart from being a nasty bug, the problem with the patch-solution, which I appreciate(!), is, that it has no contract or guarantees inside the actual svelte-code. It could break anytime. Or you might have something better to do. Or whatever.

In the end you can't simply upgrade packages without manual testing and patching - which you might eagerly need to for some reasons.
Otherwise you would have to, basically, monitor the node-count in your dynamic-pages as they may break anytime otherwise... (tbh - wtf?).

I get that this is a volunteer-driven-project and that its much hard work to develop and maintain this project (which I, again, very much appreciate!) but I'm arguing from a standpoint where one might decide to run user-facing production-code on the basis of svelte. And from that standpoint this is quite a big (undocumented) problem.

@milahu
Copy link
Contributor

milahu commented Sep 6, 2021

its much hard work to develop and maintain this project

not really : P

https://github.com/milahu/random/tree/master/svelte/patch-svelte-compiler-sources

current output
$ npm run patch
TODO patch source file: node_modules/.pnpm/[email protected]/node_modules/code-red/src/print/handlers.js
TODO patch source file: src/compiler/compile/render_dom/wrappers/Element/index.ts
TODO patch source file: src/compiler/compile/render_dom/wrappers/IfBlock.ts
TODO patch source file: src/compiler/utils/mapped_code.ts
TODO patch source file: src/compiler/compile/nodes/shared/map_children.ts
TODO patch source file: src/compiler/compile/css/Stylesheet.ts
TODO patch source file: src/compiler/preprocess/decode_sourcemap.ts
TODO patch source file: src/compiler/preprocess/index.ts

feel free to continue

@benbender
Copy link
Contributor

@milahu I was referring to svelte(kit) as a whole ;)

@pwwang
Copy link

pwwang commented Dec 17, 2021

@milahu Thanks for the effort on this. Is there a step 1-2-3 to use the code here:

https://github.com/milahu/random/tree/master/svelte/patch-svelte-compiler-sources
?

@milahu
Copy link
Contributor

milahu commented Dec 17, 2021

i made patch-svelte-compiler-sources to patch the typescript sources of svelte and its dependencies, to generate patches that can be merged upstream. but maintainers say "its a feature" so this issue is still open, almost 2 years later ...

for users of svelte, its easier to patch the bundled javascript sources of svelte (node_modules/svelte/compiler.js) with my patch-svelte-compiler.js, then make a patch with patch-package, pin svelte version in package.json, commit the patch to your repo

@quentincaffeino
Copy link

maintainers say "its a feature"

I haven't found this in this issue, was it discussed some where else?

pwwang added a commit to pwwang/pipen-report that referenced this issue Dec 17, 2021
@milahu
Copy link
Contributor

milahu commented Dec 18, 2021

maintainers say "its a feature"

I haven't found this in this issue

thats cos i made it up ; ) a more truthful summary would be:

"this is a rare issue, so it has low priority.
the issue is rare, because most people use svelte to render small pages,
and this bug is triggered only by large pages."

(again, this was never said explicitly, its just my attempt to rationalize the situation.)

if you want this fixed in svelte, feel free to continue #6716 and Rich-Harris/code-red#64

@kdheepak
Copy link

kdheepak commented Jan 2, 2022

@milahu thanks for providing the script. I'm unable to run it though, I'm getting the following error:

Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: No "exports" main defined in /Users/USER/node_modules/estree-walker/package.json
    at new NodeError (node:internal/errors:329:5)
    at throwExportsNotFound (node:internal/modules/esm/resolve:337:9)
    at packageExportsResolve (node:internal/modules/esm/resolve:526:7)
    at resolveExports (node:internal/modules/cjs/loader:473:36)
    at Function.Module._findPath (node:internal/modules/cjs/loader:513:31)
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:910:27)
    at Function.Module._load (node:internal/modules/cjs/loader:769:27)
    at Module.require (node:internal/modules/cjs/loader:996:19)
    at require (node:internal/modules/cjs/helpers:92:18)
    at Object.<anonymous> (/Users/USER/gitrepos/test/patch-svelte-compiler.cjs:29:21) {
  code: 'ERR_PACKAGE_PATH_NOT_EXPORTED'

I had to downgrade estree to v2 for this to work.

@milahu
Copy link
Contributor

milahu commented Jan 2, 2022

thanks, fixed in patch-svelte-compiler.mjs

@Conduitry
Copy link
Member

This should be good now in 3.46.4 - https://svelte.dev/repl/a9dfcc17551c4aeb95e8fe748a97061d?version=3.46.4

@benmccann
Copy link
Member

Thanks @milahu for all the investigation and driving a fix on this one!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler Changes relating to the compiler good first issue A small, encapsulated issue, ideal for a new contributor to Svelte to tackle.
Projects
None yet
Development

No branches or pull requests