[fix] prevent maximum call stack size exceeded error on large pages (sveltejs#7203)

Co-authored-by: milahu <[email protected]>
Co-authored-by: Simon <[email protected]>
Co-authored-by: Ben McCann <[email protected]>

Author: 4 people

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Svelte changelog
 
+## Unreleased
+
+* Avoid `maximum call stack size exceeded` errors on large components ([#4694](https://github.com/sveltejs/svelte/issues/4694))
+
 ## 3.46.3
 
 * Ignore whitespace in `{#each}` blocks when containing elements with `animate:` ([#5477](https://github.com/sveltejs/svelte/pull/5477))

package-lock.json

@@ -127,7 +127,7 @@
     "@typescript-eslint/parser": "^4.31.2",
     "acorn": "^8.4.1",
     "agadoo": "^1.1.0",
-    "code-red": "^0.2.4",
+    "code-red": "^0.2.5",
     "css-tree": "^1.1.2",
     "eslint": "^7.32.0",
     "eslint-plugin-import": "^2.24.2",

package.json

@@ -8,6 +8,7 @@ import { CssNode } from './interfaces';
 import hash from '../utils/hash';
 import compiler_warnings from '../compiler_warnings';
 import { extract_ignores_above_position } from '../../utils/extract_svelte_ignore';
+import { push_array } from '../../utils/push_array';
 
 function remove_css_prefix(name: string): string {
 	return name.replace(/^-((webkit)|(moz)|(o)|(ms))-/, '');
@@ -351,7 +352,7 @@ export default class Stylesheet {
 							const at_rule_declarations = node.block.children
 								.filter(node => node.type === 'Declaration')
 								.map(node => new Declaration(node));
-							atrule.declarations.push(...at_rule_declarations);
+							push_array(atrule.declarations, at_rule_declarations);
 						}
 
 						current_atrule = atrule;

src/compiler/compile/css/Stylesheet.ts

@@ -18,6 +18,7 @@ import Text from '../Text';
 import Title from '../Title';
 import Window from '../Window';
 import { TemplateNode } from '../../../interfaces';
+import { push_array } from '../../../utils/push_array';
 
 export type Children = ReturnType<typeof map_children>;
 
@@ -60,7 +61,7 @@ export default function map_children(component, parent, scope, children: Templat
 		if (use_ignores) component.pop_ignores(), ignores = [];
 
 		if (node.type === 'Comment' && node.ignores.length) {
-			ignores.push(...node.ignores);
+			push_array(ignores, node.ignores);
 		}
 
 		if (last) last.next = node;

src/compiler/compile/nodes/shared/map_children.ts

@@ -11,6 +11,7 @@ import { apply_preprocessor_sourcemap } from '../../utils/mapped_code';
 import { RawSourceMap, DecodedSourceMap } from '@ampproject/remapping/dist/types/types';
 import { flatten } from '../../utils/flatten';
 import check_enable_sourcemap from '../utils/check_enable_sourcemap';
+import { push_array } from '../../utils/push_array';
 
 export default function dom(
 	component: Component,
@@ -67,7 +68,7 @@ export default function dom(
 	// TODO the deconflicted names of blocks are reversed... should set them here
 	const blocks = renderer.blocks.slice().reverse();
 
-	body.push(...blocks.map(block => {
+	push_array(body, blocks.map(block => {
 		// TODO this is a horrible mess — renderer.blocks
 		// contains a mixture of Blocks and Nodes
 		if ((block as Block).render) return (block as Block).render();
@@ -562,7 +563,7 @@ export default function dom(
 			});
 		}
 
-		declaration.body.body.push(...accessors);
+		push_array(declaration.body.body, accessors);
 
 		body.push(declaration);
 
@@ -599,7 +600,7 @@ export default function dom(
 			}
 		`[0] as ClassDeclaration;
 
-		declaration.body.body.push(...accessors);
+		push_array(declaration.body.body, accessors);
 
 		body.push(declaration);
 	}

src/compiler/compile/render_dom/index.ts

@@ -26,6 +26,7 @@ import Action from '../../../nodes/Action';
 import MustacheTagWrapper from '../MustacheTag';
 import RawMustacheTagWrapper from '../RawMustacheTag';
 import is_dynamic from '../shared/is_dynamic';
+import { push_array } from '../../../../utils/push_array';
 
 interface BindingGroup {
 	events: string[];
@@ -597,7 +598,7 @@ export default class ElementWrapper extends Wrapper {
 		this.attributes.forEach((attribute) => {
 			if (attribute.node.name === 'class') {
 				const dependencies = attribute.node.get_dependencies();
-				this.class_dependencies.push(...dependencies);
+				push_array(this.class_dependencies, dependencies);
 			}
 		});
 

src/compiler/compile/render_dom/wrappers/Element/index.ts

@@ -10,6 +10,7 @@ import { b, x } from 'code-red';
 import { walk } from 'estree-walker';
 import { is_head } from './shared/is_head';
 import { Identifier, Node } from 'estree';
+import { push_array } from '../../../utils/push_array';
 
 function is_else_if(node: ElseBlock) {
 	return (
@@ -166,7 +167,7 @@ export default class IfBlockWrapper extends Wrapper {
 			block.has_outro_method = has_outros;
 		});
 
-		renderer.blocks.push(...blocks);
+		push_array(renderer.blocks, blocks);
 	}
 
 	render(

src/compiler/compile/render_dom/wrappers/IfBlock.ts

@@ -2,6 +2,7 @@ import { DecodedSourceMap, RawSourceMap, SourceMapLoader } from '@ampproject/rem
 import remapping from '@ampproject/remapping';
 import { SourceMap } from 'magic-string';
 import { Source, Processed } from '../preprocess/types';
+import { push_array } from './push_array';
 
 export type SourceLocation = {
 	line: number;
@@ -60,14 +61,6 @@ function merge_tables<T>(this_table: T[], other_table: T[]): [T[], number[], boo
 	return [new_table, idx_map, val_changed, idx_changed];
 }
 
-function pushArray<T>(_this: T[], other: T[]) {
-	// We use push to mutate in place for memory and perf reasons
-	// We use the for loop instead of _this.push(...other) to avoid the JS engine's function argument limit (65,535 in JavascriptCore)
-	for (let i = 0; i < other.length; i++) {
-		_this.push(other[i]);
-	}
-}
-
 export class MappedCode {
 	string: string;
 	map: DecodedSourceMap;
@@ -159,10 +152,10 @@ export class MappedCode {
 		}
 
 		// combine last line + first line
-		pushArray(m1.mappings[m1.mappings.length - 1], m2.mappings.shift());
+		push_array(m1.mappings[m1.mappings.length - 1], m2.mappings.shift());
 
 		// append other lines
-		pushArray(m1.mappings, m2.mappings);
+		push_array(m1.mappings, m2.mappings);
 
 		return this;
 	}

src/compiler/utils/mapped_code.ts

@@ -0,0 +1,12 @@
+/**
+ * Pushes all `items` into `array` using `push`, therefore mutating the array.
+ * We do this for memory and perf reasons, and because `array.push(...items)` would
+ * run into a "max call stack size exceeded" error with too many items (~65k).
+ * @param array 
+ * @param items 
+ */
+export function push_array<T>(array: T[], items: T[]): void {
+	for (let i = 0; i < items.length; i++) {
+		array.push(items[i]);
+	}
+}