Skip to content

Commit

Permalink
fix: resolve computed_prop_# collision (#8418)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ngtr6788 committed Mar 27, 2023
1 parent 95c4655 commit dadd6fe
Show file tree
Hide file tree
Showing 5 changed files with 115 additions and 21 deletions.
30 changes: 9 additions & 21 deletions src/compiler/compile/nodes/shared/Context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export type Context = DestructuredVariable | ComputedProperty;

interface ComputedProperty {
type: 'ComputedProperty';
property_name: string;
property_name: Identifier;
key: Expression | PrivateIdentifier;
}

Expand All @@ -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;
Expand All @@ -40,10 +39,6 @@ export function unpack_destructuring({
scope: TemplateScope;
component: Component;
context_rest_properties: Map<string, Node>;
// 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;

Expand Down Expand Up @@ -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') {
Expand All @@ -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({
Expand All @@ -104,8 +97,7 @@ export function unpack_destructuring({
default_modifier,
scope,
component,
context_rest_properties,
number_of_computed_props
context_rest_properties
});
}
});
Expand All @@ -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') {
Expand All @@ -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',
Expand Down Expand Up @@ -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 }
Expand All @@ -190,8 +179,7 @@ export function unpack_destructuring({
default_modifier,
scope,
component,
context_rest_properties,
number_of_computed_props
context_rest_properties
});
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
export default {
html: `
<p>4, 12, 60</p>
`,

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, `
<p>2, 11, 2</p>
<p>9506, 28811, 98</p>
`);
}
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<script>
export let promise1 = {length: 5, width: 3, height: 4};
export let promise2 = {length: 12, width: 5, height: 13};
export let permutation = [1, 2, 3];
function calculate(length, width, height) {
return {
'1-Dimensions': [length, width, height],
'2-Dimensions': [length * width, width * height, length * height],
'3-Dimensions': [length * width * height, length + width + height, length * width + width * height + length * height]
};
}
const th = 'th';
</script>

{#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) }
<p>{first}, {second}, {third}</p>
{/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) }
<p>{first}, {second}, {third}</p>
{/await}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
export default {
html: `
<button>6, 12, 8, 24</button>
<button>45, 35, 63, 315</button>
<button>60, 48, 80, 480</button>
`,

async test({ component, target, assert }) {
component.boxes = [{ length: 10, width: 20, height: 30 }];

assert.htmlEqual(target.innerHTML,
'<button>200, 600, 300, 6000</button>'
);
}
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
<script>
export let boxes = [
{length: 2, width: 3, height: 4},
{length: 9, width: 5, height: 7},
{length: 10, width: 6, height: 8}
];
function calculate(length, width, height) {
return {
twoDimensions: {
bottomArea: length * width,
sideArea1: width * height,
sideArea2: length * height
},
threeDimensions: {
volume: length * width * height
}
};
}
export let dimension = 'Dimensions';
function changeDimension() {
dimension = 'DIMENSIONS';
}
let area = 'Area';
let th = 'th';
</script>

{#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}
<button on:click={changeDimension}>{bottom}, {sideone}, {sidetwo}, {volume}</button>
{/each}

0 comments on commit dadd6fe

Please sign in to comment.