Skip to content

Commit

Permalink
[fix] introduce helper method to prevent push maximum call stack size…
Browse files Browse the repository at this point in the history
… exceeded error

Part of sveltejs#4694
Also related: Rich-Harris/code-red#71
  • Loading branch information
Simon committed Jan 29, 2022
1 parent d1b3f46 commit 7f4d0cd
Show file tree
Hide file tree
Showing 7 changed files with 27 additions and 17 deletions.
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 7f4d0cd

Please sign in to comment.