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

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

Merged
merged 4 commits into from Jan 31, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
@@ -1,5 +1,9 @@
# Svelte changelog

## Unreleased

* prevent `maximum call stack size exceeded` error on large pages ([#4694](https://github.com/sveltejs/svelte/issues/4694))
benmccann marked this conversation as resolved.
Show resolved Hide resolved

## 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]);
}
}