fix: object destructuring picks up computed properties (#8386)

fixes #6609. Prior related PR: #8357

src/compiler/compile/nodes/CatchBlock.ts

@@ -17,6 +17,7 @@ export default class CatchBlock extends AbstractBlock {
 		this.scope = scope.child();
 		if (parent.catch_node) {
 			parent.catch_contexts.forEach(context => {
+				if (context.type !== 'DestructuredVariable') return;
 				this.scope.add(context.key.name, parent.expression.dependencies, this);
 			});
 		}

src/compiler/compile/nodes/ConstTag.ts

@@ -65,6 +65,7 @@ export default class ConstTag extends Node {
 		});
 		this.expression = new Expression(this.component, this, this.scope, this.node.expression.right);
 		this.contexts.forEach(context => {
+			if (context.type !== 'DestructuredVariable') return;
 			const owner = this.scope.get_owner(context.key.name);
 			if (owner && owner.type === 'ConstTag' && owner.parent === this.parent) {
 				this.component.error(this.node, compiler_errors.invalid_const_declaration(context.key.name));

src/compiler/compile/nodes/EachBlock.ts

@@ -45,6 +45,7 @@ export default class EachBlock extends AbstractBlock {
 		unpack_destructuring({ contexts: this.contexts, node: info.context, scope, component, context_rest_properties: this.context_rest_properties });
 
 		this.contexts.forEach(context => {
+			if (context.type !== 'DestructuredVariable') return;
 			this.scope.add(context.key.name, this.expression.dependencies, this);
 		});
 

src/compiler/compile/nodes/ThenBlock.ts

@@ -17,6 +17,7 @@ export default class ThenBlock extends AbstractBlock {
 		this.scope = scope.child();
 		if (parent.then_node) {
 			parent.then_contexts.forEach(context => {
+				if (context.type !== 'DestructuredVariable') return;
 				this.scope.add(context.key.name, parent.expression.dependencies, this);
 			});
 		}

src/compiler/compile/nodes/shared/Context.ts

@@ -1,5 +1,5 @@
 import { x } from 'code-red';
-import { Node, Identifier, Expression } from 'estree';
+import { Node, Identifier, Expression, PrivateIdentifier } from 'estree';
 import { walk } from 'estree-walker';
 import is_reference, { NodeWithPropertyDefinition } from 'is-reference';
 import { clone } from '../../../utils/clone';
@@ -7,7 +7,16 @@ import Component from '../../Component';
 import flatten_reference from '../../utils/flatten_reference';
 import TemplateScope from './TemplateScope';
 
-export interface Context {
+export type Context = DestructuredVariable | ComputedProperty;
+
+interface ComputedProperty {
+	type: 'ComputedProperty';
+  property_name: string;
+  key: Expression | PrivateIdentifier;
+}
+ 
+interface DestructuredVariable {
+	type: 'DestructuredVariable'
 	key: Identifier;
 	name?: string;
 	modifier: (node: Node) => Node;
@@ -21,26 +30,33 @@ export function unpack_destructuring({
 	default_modifier = (node) => node,
 	scope,
 	component,
-	context_rest_properties
+	context_rest_properties,
+	number_of_computed_props = { n: 0 }
 }: {
 	contexts: Context[];
 	node: Node;
-	modifier?: Context['modifier'];
-	default_modifier?: Context['default_modifier'];
+	modifier?: DestructuredVariable['modifier'];
+	default_modifier?: DestructuredVariable['default_modifier'];
 	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;
 
 	if (node.type === 'Identifier') {
 		contexts.push({
+			type: 'DestructuredVariable',
 			key: node as Identifier,
 			modifier,
 			default_modifier
 		});
 	} else if (node.type === 'RestElement') {
 		contexts.push({
+			type: 'DestructuredVariable',
 			key: node.argument as Identifier,
 			modifier,
 			default_modifier
@@ -56,7 +72,8 @@ export function unpack_destructuring({
 					default_modifier,
 					scope,
 					component,
-					context_rest_properties
+					context_rest_properties,
+					number_of_computed_props
 				});
 				context_rest_properties.set((element.argument as Identifier).name, element);
 			} else if (element && element.type === 'AssignmentPattern') {
@@ -76,7 +93,8 @@ export function unpack_destructuring({
 						)}` as Node,
 					scope,
 					component,
-					context_rest_properties
+					context_rest_properties,
+					number_of_computed_props
 				});
 			} else {
 				unpack_destructuring({
@@ -86,7 +104,8 @@ export function unpack_destructuring({
 					default_modifier,
 					scope,
 					component,
-					context_rest_properties
+					context_rest_properties,
+					number_of_computed_props
 				});
 			}
 		});
@@ -105,29 +124,41 @@ export function unpack_destructuring({
 					default_modifier,
 					scope,
 					component,
-					context_rest_properties
+					context_rest_properties,
+					number_of_computed_props
 				});
 				context_rest_properties.set((property.argument as Identifier).name, property);
 			} else if (property.type === 'Property') {
 				const key = property.key;
 				const value = property.value;
 
-				let property_name: any;
 				let new_modifier: (node: Node) => Node; 
 
 				if (property.computed) {
-					// TODO: If the property is computed, ie, { [computed_key]: prop }, the computed_key can be any type of expression.
+					// e.g { [computedProperty]: ... }
+					const property_name = `computed_property_${number_of_computed_props.n}`;
+					number_of_computed_props.n += 1;
+
+					contexts.push({
+						type: 'ComputedProperty',
+						property_name,
+						key
+					});
+
+					new_modifier = (node) => x`${modifier(node)}[${property_name}]`;
+					used_properties.push(x`${property_name}`);
 				} else if (key.type === 'Identifier') {
 					// e.g. { someProperty: ... }
-				  property_name = key.name;
+					const property_name = key.name;
 					new_modifier = (node) => x`${modifier(node)}.${property_name}`;
+					used_properties.push(x`"${property_name}"`);
 				} else if (key.type === 'Literal') {
 					// e.g. { "property-in-quotes": ... } or { 14: ... }
-					property_name = key.value;
+					const property_name = key.value;
 					new_modifier = (node) => x`${modifier(node)}["${property_name}"]`;
+					used_properties.push(x`"${property_name}"`);
 				}
 				
-			  used_properties.push(x`"${property_name}"`);
 				if (value.type === 'AssignmentPattern') {
 					// e.g. { property = default } or { property: newName = default }
 					const n = contexts.length;
@@ -147,7 +178,8 @@ export function unpack_destructuring({
 							)}` as Node,
 						scope,
 						component,
-						context_rest_properties
+						context_rest_properties,
+						number_of_computed_props
 					});
 				} else {
 					// e.g. { property } or { property: newName }
@@ -158,7 +190,8 @@ export function unpack_destructuring({
 						default_modifier,
 						scope,
 						component,
-						context_rest_properties
+						context_rest_properties,
+						number_of_computed_props
 					});
 				}
 			}
@@ -174,7 +207,9 @@ function update_reference(
 ): Node {
 	const find_from_context = (node: Identifier) => {
 		for (let i = n; i < contexts.length; i++) {
-			const { key } = contexts[i];
+			const cur_context = contexts[i];
+			if (cur_context.type !== 'DestructuredVariable') continue; 
+			const { key } = cur_context;
 			if (node.name === key.name) {
 				throw new Error(`Cannot access '${node.name}' before initialization`);
 			}

src/compiler/compile/nodes/shared/Expression.ts

@@ -373,6 +373,7 @@ export default class Expression {
 								// add to get_xxx_context
 								// child_ctx[x] = function () { ... }
 								(template_scope.get_owner(deps[0]) as EachBlock).contexts.push({
+									type: 'DestructuredVariable',
 									key: func_id,
 									modifier: () => func_expression,
 									default_modifier: node => node

src/compiler/compile/render_dom/wrappers/AwaitBlock.ts

@@ -11,6 +11,7 @@ import CatchBlock from '../../nodes/CatchBlock';
 import { Context } from '../../nodes/shared/Context';
 import { Identifier, Literal, Node } from 'estree';
 import { add_const_tags, add_const_tags_context } from './shared/add_const_tags';
+import Expression from '../../nodes/shared/Expression';
 
 type Status = 'pending' | 'then' | 'catch';
 
@@ -69,6 +70,7 @@ class AwaitBlockBranch extends Wrapper {
 			this.renderer.add_to_context(this.value, true);
 		} else {
 			contexts.forEach(context => {
+				if (context.type !== 'DestructuredVariable') return;
 				this.renderer.add_to_context(context.key.name, true);
 			});
 			this.value = this.block.parent.get_unique_name('value').name;
@@ -96,7 +98,15 @@ class AwaitBlockBranch extends Wrapper {
 	}
 
 	render_get_context() {
-		const props = this.is_destructured ? this.value_contexts.map(prop => b`#ctx[${this.block.renderer.context_lookup.get(prop.key.name).index}] = ${prop.default_modifier(prop.modifier(x`#ctx[${this.value_index}]`), name => this.renderer.reference(name))};`) : null;
+		const props = this.is_destructured ? this.value_contexts.map(prop => {
+			if (prop.type === 'ComputedProperty') {
+				const expression = new Expression(this.renderer.component, this.node, this.has_consts(this.node) ? this.node.scope : null, prop.key);
+				return b`const ${prop.property_name} = ${expression.manipulate(this.block, '#ctx')};`;
+			} else {
+				const to_ctx = name => this.renderer.reference(name);
+				return b`#ctx[${this.block.renderer.context_lookup.get(prop.key.name).index}] = ${prop.default_modifier(prop.modifier(x`#ctx[${this.value_index}]`), to_ctx)};`;
+			}
+		}) : null;
 
 		const const_tags_props = this.has_consts(this.node) ? add_const_tags(this.block, this.node.const_tags, '#ctx') : null;
 

src/compiler/compile/render_dom/wrappers/EachBlock.ts

@@ -9,6 +9,7 @@ import ElseBlock from '../../nodes/ElseBlock';
 import { Identifier, Node } from 'estree';
 import get_object from '../../utils/get_object';
 import { add_const_tags, add_const_tags_context } from './shared/add_const_tags';
+import Expression from '../../nodes/shared/Expression';
 
 export class ElseBlockWrapper extends Wrapper {
 	node: ElseBlock;
@@ -86,6 +87,7 @@ export default class EachBlockWrapper extends Wrapper {
 		block.add_dependencies(dependencies);
 
 		this.node.contexts.forEach(context => {
+			if (context.type !== 'DestructuredVariable') return;
 			renderer.add_to_context(context.key.name, true);
 		});
 		add_const_tags_context(renderer, this.node.const_tags);
@@ -147,6 +149,7 @@ export default class EachBlockWrapper extends Wrapper {
 		const store = object.type === 'Identifier' && object.name[0] === '$' ? object.name.slice(1) : null;
 
 		node.contexts.forEach(prop => {
+			if (prop.type !== 'DestructuredVariable') return;
 			this.block.bindings.set(prop.key.name, {
 				object: this.vars.each_block_value,
 				property: this.index_name,
@@ -361,7 +364,15 @@ export default class EachBlockWrapper extends Wrapper {
 			this.else.fragment.render(this.else.block, null, x`#nodes` as Identifier);
 		}
 
-		this.context_props = this.node.contexts.map(prop => b`child_ctx[${renderer.context_lookup.get(prop.key.name).index}] = ${prop.default_modifier(prop.modifier(x`list[i]`), name => renderer.context_lookup.has(name) ? x`child_ctx[${renderer.context_lookup.get(name).index}]` : { type: 'Identifier', name })};`);
+		this.context_props = this.node.contexts.map(prop => {
+			if (prop.type === 'DestructuredVariable') {
+				const to_ctx = (name: string) => renderer.context_lookup.has(name) ? x`child_ctx[${renderer.context_lookup.get(name).index}]` : { type: 'Identifier', name } as Node;
+				return b`child_ctx[${renderer.context_lookup.get(prop.key.name).index}] = ${prop.default_modifier(prop.modifier(x`list[i]`), to_ctx)};`;
+			} else {
+				const expression = new Expression(this.renderer.component, this.node, this.node.scope, prop.key);
+				return b`const ${prop.property_name} = ${expression.manipulate(block, 'child_ctx')};`;
+			}
+		});
 
 		if (this.node.has_binding) this.context_props.push(b`child_ctx[${renderer.context_lookup.get(this.vars.each_block_value.name).index}] = list;`);
 		if (this.node.has_binding || this.node.has_index_binding || this.node.index) this.context_props.push(b`child_ctx[${renderer.context_lookup.get(this.index_name.name).index}] = i;`);

src/compiler/compile/render_dom/wrappers/shared/add_const_tags.ts

@@ -1,24 +1,33 @@
 import ConstTag from '../../../nodes/ConstTag';
 import Block from '../../Block';
-import { b, x } from 'code-red';
+import { b, Node, x } from 'code-red';
 import Renderer from '../../Renderer';
+import Expression from '../../../nodes/shared/Expression';
 
 export function add_const_tags(block: Block, const_tags: ConstTag[], ctx: string) {
-  const const_tags_props = [];
-  const_tags.forEach((const_tag, i) => {
-    const name = `#constants_${i}`;
-    const_tags_props.push(b`const ${name} = ${const_tag.expression.manipulate(block, ctx)}`);
-    const_tag.contexts.forEach(context => {
-      const_tags_props.push(b`${ctx}[${block.renderer.context_lookup.get(context.key.name).index}] = ${context.default_modifier(context.modifier({ type: 'Identifier', name }), name => block.renderer.context_lookup.has(name) ? x`${ctx}[${block.renderer.context_lookup.get(name).index}]` : { type: 'Identifier', name })};`);
-    });
-  });
-  return const_tags_props;
+	const const_tags_props = [];
+	const_tags.forEach((const_tag, i) => {
+		const name = `#constants_${i}`;
+		const_tags_props.push(b`const ${name} = ${const_tag.expression.manipulate(block, ctx)}`);
+		const to_ctx = (name: string) => block.renderer.context_lookup.has(name) ? x`${ctx}[${block.renderer.context_lookup.get(name).index}]` : { type: 'Identifier', name } as Node;
+
+		const_tag.contexts.forEach(context => {
+			if (context.type === 'DestructuredVariable') {
+				const_tags_props.push(b`${ctx}[${block.renderer.context_lookup.get(context.key.name).index}] = ${context.default_modifier(context.modifier({ type: 'Identifier', name }), to_ctx)}`);
+			} else {
+				const expression = new Expression(block.renderer.component, const_tag, const_tag.scope, context.key);
+				const_tags_props.push(b`const ${context.property_name} = ${expression.manipulate(block, ctx)}`);
+			}
+		});
+	});
+	return const_tags_props;
 }
 
 export function add_const_tags_context(renderer: Renderer, const_tags: ConstTag[]) {
-  const_tags.forEach(const_tag => {
-    const_tag.contexts.forEach(context => {
-      renderer.add_to_context(context.key.name, true);
-    });
-  });
+	const_tags.forEach(const_tag => {
+		const_tag.contexts.forEach(context => {
+			if (context.type !== 'DestructuredVariable') return;
+			renderer.add_to_context(context.key.name, true);
+		});
+	});
 }

test/runtime/samples/await-then-destruct-computed-props/_config.js

@@ -0,0 +1,32 @@
+export default {
+	async test({ assert, component, target }) {
+		await Promise.resolve();
+
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+			<p>propA: 3</p>
+			<p>propB: 7</p>
+			<p>num: 3</p>
+			<p>rest: {"prop3":{"prop9":9,"prop10":10}}</p>
+			<p>propZ: 5</p>
+			<p>propY: 6</p>
+			<p>rest: {"propX":7,"propW":8}</p>
+			`
+		);
+
+		await (component.object = Promise.resolve({ prop1: 'one', prop2: 'two', prop3: { prop7: 'seven' }, prop4: { prop10: 'ten' }}));
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+			<p>propA: seven</p>
+			<p>propB: ten</p>
+			<p>num: 5</p>
+			<p>rest: {"prop1":"one","prop2":"two"}</p>
+			<p>propZ: 5</p>
+			<p>propY: 6</p>
+			<p>rest: {"propX":7,"propW":8}</p>
+			`
+		);
+	}
+};

test/runtime/samples/await-then-destruct-computed-props/main.svelte

@@ -0,0 +1,23 @@
+<script>
+	export let object = Promise.resolve({ prop1: { prop4: 2, prop5: 3 }, prop2: { prop6: 5, prop7: 6, prop8: 7 }, prop3: { prop9: 9, prop10: 10 } });
+	const objectReject = Promise.reject({ propZ: 5, propY: 6, propX: 7, propW: 8 });
+
+	let num = 1;
+	const prop = 'prop';
+</script>
+
+{#await object then { [`prop${num++}`]: { [`prop${num + 3}`]: propA }, [`prop${num++}`]: { [`prop${num + 5}`]: propB }, ...rest }}
+	<p>propA: {propA}</p>
+	<p>propB: {propB}</p>
+	<p>num: {num}</p>
+	<p>rest: {JSON.stringify(rest)}</p>
+{/await}
+
+{#await objectReject then value}
+	resolved
+{:catch { [`${prop}Z`]: propZ, [`${prop}Y`]: propY, ...rest }}
+	<p>propZ: {propZ}</p>
+	<p>propY: {propY}</p>
+	<p>rest: {JSON.stringify(rest)}</p>
+{/await}
+

test/runtime/samples/const-tag-await-then-destructuring-computed-props/_config.js

@@ -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>
+		`);
+	}
+};

test/runtime/samples/const-tag-await-then-destructuring-computed-props/main.svelte

@@ -0,0 +1,25 @@
+<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]
+		};
+	}
+</script>
+
+{#await promise1 then { length, width, height }}
+	{@const [a, b, 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 { length, width, height }}
+	{@const [a, b, 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}

test/runtime/samples/const-tag-each-destructure-computed-props/_config.js

@@ -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>'
+		);
+	}
+};

test/runtime/samples/const-tag-each-destructure-computed-props/main.svelte

@@ -0,0 +1,42 @@
+<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';
+</script>
+
+{#each boxes as { length, width, height }}
+	{@const {
+		[`two${dimension}`]: { 
+			i = 1, 
+			[`bottom${area}`]: bottom, 
+			[`side${area}${i++}`]: sideone, 
+			[`side${area}${i++}`]: sidetwo 
+		},
+		[`three${dimension}`]: {
+			volume
+		}
+	} = calculate(length, width, height)}
+	<button on:click={changeDimension}>{bottom}, {sideone}, {sidetwo}, {volume}</button>
+{/each}

test/runtime/samples/each-block-destructured-array-computed-props/_config.js

@@ -0,0 +1,24 @@
+export default {
+	props: {
+		array: [
+			[1, 2, 3, 4, 5],
+			[6, 7, 8],
+			[9, 10, 11, 12],
+			[13, 14, 15, 16, 17, 18, 19, 20, 21, 22]
+		]
+	},
+
+	html: `
+		<p>First: 1, Half: 3, Last: 5, Length: 5</p>
+		<p>First: 6, Half: 7, Last: 8, Length: 3</p>
+		<p>First: 9, Half: 11, Last: 12, Length: 4</p>
+		<p>First: 13, Half: 18, Last: 22, Length: 10</p>
+	`,
+
+	test({ assert, component, target }) {
+		component.array = [[23, 24, 25, 26, 27, 28, 29]];
+		assert.htmlEqual( target.innerHTML, `
+			<p>First: 23, Half: 26, Last: 29, Length: 7</p>
+		`);
+	}
+};

test/runtime/samples/each-block-destructured-array-computed-props/main.svelte

@@ -0,0 +1,7 @@
+<script>
+	export let array;
+</script>
+
+{#each array as { 0: first, length, [length - 1]: last, [Math.floor(length / 2)]: half }}
+	<p>First: {first}, Half: {half}, Last: {last}, Length: {length}</p>
+{/each}

test/runtime/samples/each-block-destructured-object-computed-props/_config.js

@@ -0,0 +1,26 @@
+export default {
+	props: {
+		firstString: 'cats',
+		secondString: 'dogs',
+		objectsArray: [
+			{ dogs: 'woof', cats: 'meow', stac: 'stack', DOGS: 'WOOF' },
+			{ dogs: 'A German sheppard', cats: 'A tailless cat', stac: 'A jenga tower', DOGS: 'A GERMAN SHEPPARD' },
+			{ dogs: 'dogs', cats: 'cats', stac: 'stac', DOGS: 'DOGS' }
+		]
+	},
+
+	html: `
+		<p>cats: meow</p>
+		<p>dogs: woof</p>
+		<p>stac: stack</p>
+		<p>DOGS: WOOF</p> 
+		<p>cats: A tailless cat</p>
+		<p>dogs: A German sheppard</p>
+		<p>stac: A jenga tower</p>
+		<p>DOGS: A GERMAN SHEPPARD</p> 
+		<p>cats: cats</p>
+		<p>dogs: dogs</p>
+		<p>stac: stac</p>
+		<p>DOGS: DOGS</p> 
+	`
+};

test/runtime/samples/each-block-destructured-object-computed-props/main.svelte

@@ -0,0 +1,12 @@
+<script>
+	export let objectsArray;
+	export let firstString;
+	export let secondString;
+</script>
+
+{#each objectsArray as { [firstString]: firstProp, [secondString]: secondProp, [firstString.split('').reverse().join('')]: reverseFirst, [secondString.toUpperCase()]: upperSecond } }
+	<p>{firstString}: {firstProp}</p>
+  <p>{secondString}: {secondProp}</p>
+	<p>{firstString.split('').reverse().join('')}: {reverseFirst}</p>
+	<p>{secondString.toUpperCase()}: {upperSecond}</p>
+{/each}