From dadd6fe9455f0f2e7a7d1dce365e50d64953bc3e Mon Sep 17 00:00:00 2001 From: Nguyen Tran <88808276+ngtr6788@users.noreply.github.com> Date: Mon, 27 Mar 2023 04:15:39 -0400 Subject: [PATCH] fix: resolve computed_prop_# collision (#8418) Fixes #8417 The issue is that unpack_destructuring in each blocks, await blocks, and @const tags were making computed_props independently. This causes computed_props_# to conflict when each blocks were used with @const tags, or await blocks and @const tags, or consecutive @const tags together. Therefore, one solution is to use component.get_unique_name to, well, make unique names and never get conflicts. --- src/compiler/compile/nodes/shared/Context.ts | 30 ++++--------- .../_config.js | 20 +++++++++ .../main.svelte | 27 ++++++++++++ .../_config.js | 15 +++++++ .../main.svelte | 44 +++++++++++++++++++ 5 files changed, 115 insertions(+), 21 deletions(-) create mode 100644 test/runtime/samples/const-tag-await-then-destructuring-computed-in-computed/_config.js create mode 100644 test/runtime/samples/const-tag-await-then-destructuring-computed-in-computed/main.svelte create mode 100644 test/runtime/samples/const-tag-each-destructure-computed-in-computed/_config.js create mode 100644 test/runtime/samples/const-tag-each-destructure-computed-in-computed/main.svelte diff --git a/src/compiler/compile/nodes/shared/Context.ts b/src/compiler/compile/nodes/shared/Context.ts index 3957ed2173..76ac895681 100644 --- a/src/compiler/compile/nodes/shared/Context.ts +++ b/src/compiler/compile/nodes/shared/Context.ts @@ -11,7 +11,7 @@ export type Context = DestructuredVariable | ComputedProperty; interface ComputedProperty { type: 'ComputedProperty'; - property_name: string; + property_name: Identifier; key: Expression | PrivateIdentifier; } @@ -30,8 +30,7 @@ export function unpack_destructuring({ default_modifier = (node) => node, scope, component, - context_rest_properties, - number_of_computed_props = { n: 0 } + context_rest_properties }: { contexts: Context[]; node: Node; @@ -40,10 +39,6 @@ export function unpack_destructuring({ scope: TemplateScope; component: Component; context_rest_properties: Map; - // we want to pass this by reference, as a sort of global variable, because - // if we pass this by value, we could get computed_property_# variable collisions - // when we deal with nested object destructuring - number_of_computed_props?: { n: number }; }) { if (!node) return; @@ -72,8 +67,7 @@ export function unpack_destructuring({ default_modifier, scope, component, - context_rest_properties, - number_of_computed_props + context_rest_properties }); context_rest_properties.set((element.argument as Identifier).name, element); } else if (element && element.type === 'AssignmentPattern') { @@ -93,8 +87,7 @@ export function unpack_destructuring({ )}` as Node, scope, component, - context_rest_properties, - number_of_computed_props + context_rest_properties }); } else { unpack_destructuring({ @@ -104,8 +97,7 @@ export function unpack_destructuring({ default_modifier, scope, component, - context_rest_properties, - number_of_computed_props + context_rest_properties }); } }); @@ -124,8 +116,7 @@ export function unpack_destructuring({ default_modifier, scope, component, - context_rest_properties, - number_of_computed_props + context_rest_properties }); context_rest_properties.set((property.argument as Identifier).name, property); } else if (property.type === 'Property') { @@ -136,8 +127,7 @@ export function unpack_destructuring({ if (property.computed) { // e.g { [computedProperty]: ... } - const property_name = `computed_property_${number_of_computed_props.n}`; - number_of_computed_props.n += 1; + const property_name = component.get_unique_name('computed_property'); contexts.push({ type: 'ComputedProperty', @@ -178,8 +168,7 @@ export function unpack_destructuring({ )}` as Node, scope, component, - context_rest_properties, - number_of_computed_props + context_rest_properties }); } else { // e.g. { property } or { property: newName } @@ -190,8 +179,7 @@ export function unpack_destructuring({ default_modifier, scope, component, - context_rest_properties, - number_of_computed_props + context_rest_properties }); } } diff --git a/test/runtime/samples/const-tag-await-then-destructuring-computed-in-computed/_config.js b/test/runtime/samples/const-tag-await-then-destructuring-computed-in-computed/_config.js new file mode 100644 index 0000000000..9ec4a3e3e6 --- /dev/null +++ b/test/runtime/samples/const-tag-await-then-destructuring-computed-in-computed/_config.js @@ -0,0 +1,20 @@ +export default { + html: ` +

4, 12, 60

+ `, + + async test({ component, target, assert }) { + component.permutation = [2, 3, 1]; + await (component.promise1 = Promise.resolve({length: 1, width: 2, height: 3})); + try { + await (component.promise2 = Promise.reject({length: 97, width: 98, height: 99})); + } catch (e) { + // nothing + } + + assert.htmlEqual(target.innerHTML, ` +

2, 11, 2

+

9506, 28811, 98

+ `); + } +}; diff --git a/test/runtime/samples/const-tag-await-then-destructuring-computed-in-computed/main.svelte b/test/runtime/samples/const-tag-await-then-destructuring-computed-in-computed/main.svelte new file mode 100644 index 0000000000..285fc44348 --- /dev/null +++ b/test/runtime/samples/const-tag-await-then-destructuring-computed-in-computed/main.svelte @@ -0,0 +1,27 @@ + + +{#await promise1 then { length, width, height }} + {@const { [0]: a, [1]: b, [2]: c } = permutation} + {@const { [`${a}-Dimensions`]: { [c - 1]: first }, [`${b}-Dimensions`]: { [b - 1]: second }, [`${c}-Dimensions`]: { [a - 1]: third } } = calculate(length, width, height) } +

{first}, {second}, {third}

+{/await} + +{#await promise2 catch { [`leng${th}`]: l, [`wid${th}`]: w, height: h }} + {@const [a, b, c] = permutation} + {@const { [`${a}-Dimensions`]: { [c - 1]: first }, [`${b}-Dimensions`]: { [b - 1]: second }, [`${c}-Dimensions`]: { [a - 1]: third } } = calculate(l, w, h) } +

{first}, {second}, {third}

+{/await} diff --git a/test/runtime/samples/const-tag-each-destructure-computed-in-computed/_config.js b/test/runtime/samples/const-tag-each-destructure-computed-in-computed/_config.js new file mode 100644 index 0000000000..c299f19da6 --- /dev/null +++ b/test/runtime/samples/const-tag-each-destructure-computed-in-computed/_config.js @@ -0,0 +1,15 @@ +export default { + html: ` + + + + `, + + async test({ component, target, assert }) { + component.boxes = [{ length: 10, width: 20, height: 30 }]; + + assert.htmlEqual(target.innerHTML, + '' + ); + } +}; diff --git a/test/runtime/samples/const-tag-each-destructure-computed-in-computed/main.svelte b/test/runtime/samples/const-tag-each-destructure-computed-in-computed/main.svelte new file mode 100644 index 0000000000..26dc5557ed --- /dev/null +++ b/test/runtime/samples/const-tag-each-destructure-computed-in-computed/main.svelte @@ -0,0 +1,44 @@ + + +{#each boxes as { [`leng${th}`]: length, [`wid${th}`]: width, height }} + {@const { + [`two${dimension}`]: areas, + [`three${dimension}`]: { + volume + } + } = calculate(length, width, height)} + {@const { + i = 1, + [`bottom${area}`]: bottom, + [`side${area}${i++}`]: sideone, + [`side${area}${i++}`]: sidetwo + } = areas} + +{/each}