From 2c9d864e33feba60d5f0b47d7b71f098815814a0 Mon Sep 17 00:00:00 2001 From: Tan Li Hau Date: Sat, 7 Dec 2019 14:42:36 +0800 Subject: [PATCH] fix: loop-guard scope leak --- src/compiler/compile/Component.ts | 64 ++++++------ test/js/samples/loop-protect/expected.js | 99 ++++++++++++++----- test/js/samples/loop-protect/input.svelte | 13 ++- .../loop-protect-inner-function/_config.js | 7 ++ .../loop-protect-inner-function/main.svelte | 7 ++ 5 files changed, 127 insertions(+), 63 deletions(-) create mode 100644 test/runtime/samples/loop-protect-inner-function/_config.js create mode 100644 test/runtime/samples/loop-protect-inner-function/main.svelte diff --git a/src/compiler/compile/Component.ts b/src/compiler/compile/Component.ts index de5400367d..1c4b36360c 100644 --- a/src/compiler/compile/Component.ts +++ b/src/compiler/compile/Component.ts @@ -344,7 +344,7 @@ export default class Component { }; } - get_unique_name(name: string): Identifier { + get_unique_name(name: string, scope?: Scope): Identifier { if (test) name = `${name}$`; let alias = name; for ( @@ -352,7 +352,8 @@ export default class Component { reserved.has(alias) || this.var_lookup.has(alias) || this.used_names.has(alias) || - this.globally_used_names.has(alias); + this.globally_used_names.has(alias) || + (scope && scope.has(alias)); alias = `${name}_${i++}` ); this.used_names.add(alias); @@ -707,8 +708,7 @@ export default class Component { const remove = (parent, prop, index) => { to_remove.unshift([parent, prop, index]); }; - - const to_insert = new Map(); + let scope_updated = false; walk(content, { enter(node, parent, prop, index) { @@ -735,37 +735,21 @@ export default class Component { } component.warn_on_undefined_store_value_references(node, parent, scope); + }, + leave(node) { + // do it on leave, to prevent infinite loop if (component.compile_options.dev && component.compile_options.loopGuardTimeout > 0) { - const to_insert_for_loop_protect = component.loop_protect(node, prop, index, component.compile_options.loopGuardTimeout); - if (to_insert_for_loop_protect) { - if (!Array.isArray(parent[prop])) { - parent[prop] = { - type: 'BlockStatement', - body: [to_insert_for_loop_protect.node, node], - }; - } else { - // can't insert directly, will screw up the index in the for-loop of estree-walker - if (!to_insert.has(parent)) { - to_insert.set(parent, []); - } - to_insert.get(parent).push(to_insert_for_loop_protect); - } + const to_replace_for_loop_protect = component.loop_protect(node, scope, component.compile_options.loopGuardTimeout); + if (to_replace_for_loop_protect) { + this.replace(to_replace_for_loop_protect); + scope_updated = true; } } - }, - leave(node) { if (map.has(node)) { scope = scope.parent; - } - if (to_insert.has(node)) { - const nodes_to_insert = to_insert.get(node); - for (const { index, prop, node: node_to_insert } of nodes_to_insert.reverse()) { - node[prop].splice(index, 0, node_to_insert); - } - to_insert.delete(node); - } + } }, }); @@ -778,6 +762,12 @@ export default class Component { } } } + + if (scope_updated) { + const { scope, map } = create_scopes(script.content); + this.instance_scope = scope; + this.instance_scope_map = map; + } } track_references_and_mutations() { @@ -849,15 +839,12 @@ export default class Component { } } - loop_protect(node, prop, index, timeout) { + loop_protect(node, scope: Scope, timeout: number): Node | null { if (node.type === 'WhileStatement' || node.type === 'ForStatement' || node.type === 'DoWhileStatement') { - const guard = this.get_unique_name('guard'); - this.add_var({ - name: guard.name, - internal: true, - }); + const guard = this.get_unique_name('guard', scope); + this.used_names.add(guard.name); const before = b`const ${guard} = @loop_guard(${timeout})`; const inside = b`${guard}();`; @@ -870,7 +857,14 @@ export default class Component { }; } node.body.body.push(inside[0]); - return { index, prop, node: before[0] }; + + return { + type: 'BlockStatement', + body: [ + before[0], + node, + ], + }; } return null; } diff --git a/test/js/samples/loop-protect/expected.js b/test/js/samples/loop-protect/expected.js index 093cccf63c..554ccf23b1 100644 --- a/test/js/samples/loop-protect/expected.js +++ b/test/js/samples/loop-protect/expected.js @@ -1,8 +1,13 @@ /* generated by Svelte vX.Y.Z */ import { SvelteComponentDev, + add_location, + binding_callbacks, + detach_dev, dispatch_dev, + element, init, + insert_dev, loop_guard, noop, safe_not_equal @@ -11,16 +16,27 @@ import { const file = undefined; function create_fragment(ctx) { + let div; + const block = { - c: noop, + c: function create() { + div = element("div"); + add_location(div, file, 22, 0, 288); + }, l: function claim(nodes) { throw new Error("options.hydrate only works if the component was compiled with the `hydratable: true` option"); }, - m: noop, + m: function mount(target, anchor) { + insert_dev(target, div, anchor); + /*div_binding*/ ctx[1](div); + }, p: noop, i: noop, o: noop, - d: noop + d: function destroy(detaching) { + if (detaching) detach_dev(div); + /*div_binding*/ ctx[1](null); + } }; dispatch_dev("SvelteRegisterBlock", { @@ -34,62 +50,91 @@ function create_fragment(ctx) { return block; } -function instance($$self) { - const guard = loop_guard(100); +function foo() { + const guard = "foo"; + + { + const guard_1 = loop_guard(100); - while (true) { - foo(); - guard(); + while (true) { + console.log(guard); + guard_1(); + } } +} + +function instance($$self, $$props, $$invalidate) { + let node; - const guard_1 = loop_guard(100); + { + const guard = loop_guard(100); - for (; ; ) { - foo(); - guard_1(); + while (true) { + foo(); + guard(); + } } - const guard_2 = loop_guard(100); + { + const guard_2 = loop_guard(100); - while (true) { - foo(); - guard_2(); + for (; ; ) { + foo(); + guard_2(); + } } - const guard_4 = loop_guard(100); + { + const guard_3 = loop_guard(100); - do { - foo(); - guard_4(); - } while (true); + while (true) { + foo(); + guard_3(); + } + } + + { + const guard_5 = loop_guard(100); + + do { + foo(); + guard_5(); + } while (true); + } + + function div_binding($$value) { + binding_callbacks[$$value ? "unshift" : "push"](() => { + $$invalidate(0, node = $$value); + }); + } $$self.$capture_state = () => { return {}; }; $$self.$inject_state = $$props => { - + if ("node" in $$props) $$invalidate(0, node = $$props.node); }; $: { - const guard_3 = loop_guard(100); + const guard_4 = loop_guard(100); while (true) { foo(); - guard_3(); + guard_4(); } } $: { - const guard_5 = loop_guard(100); + const guard_6 = loop_guard(100); do { foo(); - guard_5(); + guard_6(); } while (true); } - return []; + return [node, div_binding]; } class Component extends SvelteComponentDev { diff --git a/test/js/samples/loop-protect/input.svelte b/test/js/samples/loop-protect/input.svelte index c39ea75f72..daac6ab1c4 100644 --- a/test/js/samples/loop-protect/input.svelte +++ b/test/js/samples/loop-protect/input.svelte @@ -1,4 +1,13 @@ \ No newline at end of file + + +
\ No newline at end of file diff --git a/test/runtime/samples/loop-protect-inner-function/_config.js b/test/runtime/samples/loop-protect-inner-function/_config.js new file mode 100644 index 0000000000..862d4f4c0f --- /dev/null +++ b/test/runtime/samples/loop-protect-inner-function/_config.js @@ -0,0 +1,7 @@ +export default { + html: '
', + compileOptions: { + dev: true, + loopGuardTimeout: 100, + } +}; diff --git a/test/runtime/samples/loop-protect-inner-function/main.svelte b/test/runtime/samples/loop-protect-inner-function/main.svelte new file mode 100644 index 0000000000..cf5350efde --- /dev/null +++ b/test/runtime/samples/loop-protect-inner-function/main.svelte @@ -0,0 +1,7 @@ + +
\ No newline at end of file