From 7f4d0cda74ff1e6426c1e5596db3bf5dc58adb3d Mon Sep 17 00:00:00 2001 From: Simon Date: Sat, 29 Jan 2022 23:03:25 +0100 Subject: [PATCH] [fix] introduce helper method to prevent push maximum call stack size exceeded error Part of #4694 Also related: https://github.com/Rich-Harris/code-red/pull/71 --- src/compiler/compile/css/Stylesheet.ts | 3 ++- src/compiler/compile/nodes/shared/map_children.ts | 3 ++- src/compiler/compile/render_dom/index.ts | 7 ++++--- .../compile/render_dom/wrappers/Element/index.ts | 3 ++- src/compiler/compile/render_dom/wrappers/IfBlock.ts | 3 ++- src/compiler/utils/mapped_code.ts | 13 +++---------- src/compiler/utils/push_array.ts | 12 ++++++++++++ 7 files changed, 27 insertions(+), 17 deletions(-) create mode 100644 src/compiler/utils/push_array.ts diff --git a/src/compiler/compile/css/Stylesheet.ts b/src/compiler/compile/css/Stylesheet.ts index 39b7be9db6f..8f88f0cbd40 100644 --- a/src/compiler/compile/css/Stylesheet.ts +++ b/src/compiler/compile/css/Stylesheet.ts @@ -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; diff --git a/src/compiler/compile/nodes/shared/map_children.ts b/src/compiler/compile/nodes/shared/map_children.ts index 8fe53088bd5..e6ad1d7f6ee 100644 --- a/src/compiler/compile/nodes/shared/map_children.ts +++ b/src/compiler/compile/nodes/shared/map_children.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; @@ -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; diff --git a/src/compiler/compile/render_dom/index.ts b/src/compiler/compile/render_dom/index.ts index 89af0c297c5..9d9699bdbf6 100644 --- a/src/compiler/compile/render_dom/index.ts +++ b/src/compiler/compile/render_dom/index.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); } diff --git a/src/compiler/compile/render_dom/wrappers/Element/index.ts b/src/compiler/compile/render_dom/wrappers/Element/index.ts index 80b4c7ca7a0..eb57233ed0a 100644 --- a/src/compiler/compile/render_dom/wrappers/Element/index.ts +++ b/src/compiler/compile/render_dom/wrappers/Element/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); } }); diff --git a/src/compiler/compile/render_dom/wrappers/IfBlock.ts b/src/compiler/compile/render_dom/wrappers/IfBlock.ts index 01d49ac9dd4..e94404290f2 100644 --- a/src/compiler/compile/render_dom/wrappers/IfBlock.ts +++ b/src/compiler/compile/render_dom/wrappers/IfBlock.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( diff --git a/src/compiler/utils/mapped_code.ts b/src/compiler/utils/mapped_code.ts index 58f44d7b8c9..04fa05be9ad 100644 --- a/src/compiler/utils/mapped_code.ts +++ b/src/compiler/utils/mapped_code.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(this_table: T[], other_table: T[]): [T[], number[], boo return [new_table, idx_map, val_changed, idx_changed]; } -function pushArray(_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; } diff --git a/src/compiler/utils/push_array.ts b/src/compiler/utils/push_array.ts new file mode 100644 index 00000000000..d2c0913c920 --- /dev/null +++ b/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(array: T[], items: T[]): void { + for (let i = 0; i < items.length; i++) { + array.push(items[i]); + } +}