Skip to content

Commit

Permalink
[fix] prevent maximum call stack size exceeded error on large pages (s…
Browse files Browse the repository at this point in the history
…veltejs#7203)

Co-authored-by: milahu <milahu@gmail.com>
Co-authored-by: Simon <simon.holthausen@accso.de>
Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
  • Loading branch information
4 people authored and nevilm-lt committed Apr 21, 2022
1 parent 58b81f0 commit 33eb9a3
Show file tree
Hide file tree
Showing 10 changed files with 39 additions and 26 deletions.
4 changes: 4 additions & 0 deletions 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))
Expand Down
15 changes: 7 additions & 8 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Expand Up @@ -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",
Expand Down
3 changes: 2 additions & 1 deletion src/compiler/compile/css/Stylesheet.ts
Expand Up @@ -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))-/, '');
Expand Down Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion src/compiler/compile/nodes/shared/map_children.ts
Expand Up @@ -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>;

Expand Down Expand Up @@ -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;
Expand Down
7 changes: 4 additions & 3 deletions src/compiler/compile/render_dom/index.ts
Expand Up @@ -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,
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -562,7 +563,7 @@ export default function dom(
});
}

declaration.body.body.push(...accessors);
push_array(declaration.body.body, accessors);

body.push(declaration);

Expand Down Expand Up @@ -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);
}
Expand Down
3 changes: 2 additions & 1 deletion src/compiler/compile/render_dom/wrappers/Element/index.ts
Expand Up @@ -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[];
Expand Down Expand Up @@ -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);
}
});

Expand Down
3 changes: 2 additions & 1 deletion src/compiler/compile/render_dom/wrappers/IfBlock.ts
Expand Up @@ -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 (
Expand Down Expand Up @@ -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(
Expand Down
13 changes: 3 additions & 10 deletions src/compiler/utils/mapped_code.ts
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
12 changes: 12 additions & 0 deletions src/compiler/utils/push_array.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]);
}
}

0 comments on commit 33eb9a3

Please sign in to comment.