allow destructured defaults to refer to variables (#5986)

tanhauhau · habibrosyad · Conduitry · web-flow · commit d17a90cc441e · 2021-02-25T11:36:44.000-05:00
Co-authored-by: M. Habib Rosyad &lt;habib@volantis.io&gt;
Co-authored-by: Conduitry &lt;git@chor.date&gt;
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,7 @@
 ## Unreleased
 
 * In custom elements, call `onMount` functions when connecting and clean up when disconnecting ([#1152](https://github.com/sveltejs/svelte/issues/1152), [#2227](https://github.com/sveltejs/svelte/issues/2227), [#4522](https://github.com/sveltejs/svelte/pull/4522))
+* Allow destructured defaults to refer to other variables ([#5066](https://github.com/sveltejs/svelte/issues/5066))
 * Do not emit `contextual-store` warnings for function parameters or declared variables ([#6008](https://github.com/sveltejs/svelte/pull/6008))
 
 ## 3.32.3
diff --git a/src/compiler/compile/nodes/AwaitBlock.ts b/src/compiler/compile/nodes/AwaitBlock.ts
@@ -33,12 +33,12 @@ export default class AwaitBlock extends Node {
 
 		if (this.then_node) {
 			this.then_contexts = [];
-			unpack_destructuring(this.then_contexts, info.value, node => node);
+			unpack_destructuring(this.then_contexts, info.value);
 		}
 
 		if (this.catch_node) {
 			this.catch_contexts = [];
-			unpack_destructuring(this.catch_contexts, info.error, node => node);
+			unpack_destructuring(this.catch_contexts, info.error);
 		}
 
 		this.pending = new PendingBlock(component, this, scope, info.pending);
diff --git a/src/compiler/compile/nodes/EachBlock.ts b/src/compiler/compile/nodes/EachBlock.ts
@@ -38,7 +38,7 @@ export default class EachBlock extends AbstractBlock {
 		this.scope = scope.child();
 
 		this.contexts = [];
-		unpack_destructuring(this.contexts, info.context, node => node);
+		unpack_destructuring(this.contexts, info.context);
 
 		this.contexts.forEach(context => {
 			this.scope.add(context.key.name, this.expression.dependencies, this);
diff --git a/src/compiler/compile/nodes/shared/Context.ts b/src/compiler/compile/nodes/shared/Context.ts
@@ -1,33 +1,40 @@
 import { x } from 'code-red';
-import { Node, Identifier } from 'estree';
+import { Node, Identifier, Expression } from 'estree';
+import { walk } from 'estree-walker';
+import is_reference from 'is-reference';
 
 export interface Context {
 	key: Identifier;
 	name?: string;
 	modifier: (node: Node) => Node;
+	default_modifier: (node: Node, to_ctx: (name: string) => Node) => Node;
 }
 
-export function unpack_destructuring(contexts: Context[], node: Node, modifier: (node: Node) => Node) {
+export function unpack_destructuring(contexts: Context[], node: Node, modifier: Context['modifier'] = node => node, default_modifier: Context['default_modifier'] = node => node) {
 	if (!node) return;
 
 	if (node.type === 'Identifier') {
 		contexts.push({
 			key: node as Identifier,
-			modifier
+			modifier,
+			default_modifier
 		});
 	} else if (node.type === 'RestElement') {
 		contexts.push({
 			key: node.argument as Identifier,
-			modifier
+			modifier,
+			default_modifier
 		});
 	} else if (node.type === 'ArrayPattern') {
 		node.elements.forEach((element, i) => {
 			if (element && element.type === 'RestElement') {
-				unpack_destructuring(contexts, element, node => x`${modifier(node)}.slice(${i})` as Node);
+				unpack_destructuring(contexts, element, node => x`${modifier(node)}.slice(${i})` as Node, default_modifier);
 			} else if (element && element.type === 'AssignmentPattern') {
-				unpack_destructuring(contexts, element.left, node => x`${modifier(node)}[${i}] !== undefined ? ${modifier(node)}[${i}] : ${element.right}` as Node);
+				const n = contexts.length;
+
+				unpack_destructuring(contexts, element.left, node => x`${modifier(node)}[${i}]`, (node, to_ctx) => x`${node} !== undefined ? ${node} : ${update_reference(contexts, n, element.right, to_ctx)}` as Node);
 			} else {
-				unpack_destructuring(contexts, element, node => x`${modifier(node)}[${i}]` as Node);
+				unpack_destructuring(contexts, element, node => x`${modifier(node)}[${i}]` as Node, default_modifier);
 			}
 		});
 	} else if (node.type === 'ObjectPattern') {
@@ -38,19 +45,51 @@ export function unpack_destructuring(contexts: Context[], node: Node, modifier:
 				unpack_destructuring(
 					contexts,
 					property.argument,
-					node => x`@object_without_properties(${modifier(node)}, [${used_properties}])` as Node
+					node => x`@object_without_properties(${modifier(node)}, [${used_properties}])` as Node,
+					default_modifier
 				);
 			} else {
 				const key = property.key as Identifier;
 				const value = property.value;
 
-				used_properties.push(x`"${(key as Identifier).name}"`);
+				used_properties.push(x`"${key.name}"`);
 				if (value.type === 'AssignmentPattern') {
-					unpack_destructuring(contexts, value.left, node => x`${modifier(node)}.${key.name} !== undefined ? ${modifier(node)}.${key.name} : ${value.right}` as Node);
+					const n = contexts.length;
+
+					unpack_destructuring(contexts, value.left, node => x`${modifier(node)}.${key.name}`, (node, to_ctx) => x`${node} !== undefined ? ${node} : ${update_reference(contexts, n, value.right, to_ctx)}` as Node);
 				} else {
-					unpack_destructuring(contexts, value, node => x`${modifier(node)}.${key.name}` as Node);
+					unpack_destructuring(contexts, value, node => x`${modifier(node)}.${key.name}` as Node, default_modifier);
 				}
 			}
 		});
 	}
 }
+
+function update_reference(contexts: Context[], n: number, expression: Expression, to_ctx: (name: string) => Node): Node {
+	const find_from_context = (node: Identifier) => {
+		for (let i = n; i < contexts.length; i++) {
+			const { key } = contexts[i];
+			if (node.name === key.name) {
+				throw new Error(`Cannot access '${node.name}' before initialization`);
+			}
+		}
+		return to_ctx(node.name);
+	};
+
+	if (expression.type === 'Identifier') {
+		return find_from_context(expression);
+	}
+
+	// NOTE: avoid unnecessary deep clone?
+	expression = JSON.parse(JSON.stringify(expression)) as Expression;
+	walk(expression, {
+		enter(node, parent: Node) {
+			if (is_reference(node, parent)) {
+				this.replace(find_from_context(node as Identifier));
+				this.skip();
+			}
+		}
+	});
+
+	return expression;
+}
diff --git a/src/compiler/compile/nodes/shared/Expression.ts b/src/compiler/compile/nodes/shared/Expression.ts
@@ -325,7 +325,8 @@ export default class Expression {
 								// child_ctx[x] = function () { ... }
 								(template_scope.get_owner(deps[0]) as EachBlock).contexts.push({
 									key: func_id,
-									modifier: () => func_expression
+									modifier: () => func_expression,
+									default_modifier: node => node
 								});
 								this.replace(block.renderer.reference(func_id));
 							}
diff --git a/src/compiler/compile/render_dom/Block.ts b/src/compiler/compile/render_dom/Block.ts
@@ -9,7 +9,6 @@ export interface Bindings {
 	property: Identifier;
 	snippet: Node;
 	store: string;
-	tail: Node;
 	modifier: (node: Node) => Node;
 }
 
diff --git a/src/compiler/compile/render_dom/wrappers/AwaitBlock.ts b/src/compiler/compile/render_dom/wrappers/AwaitBlock.ts
@@ -87,7 +87,7 @@ class AwaitBlockBranch extends Wrapper {
 	}
 
 	render_destructure() {
-		const props = this.value_contexts.map(prop => b`#ctx[${this.block.renderer.context_lookup.get(prop.key.name).index}] = ${prop.modifier(x`#ctx[${this.value_index}]`)};`);
+		const props = 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))};`);
 		const get_context = this.block.renderer.component.get_unique_name(`get_${this.status}_context`);
 		this.block.renderer.blocks.push(b`
 			function ${get_context}(#ctx) {
diff --git a/src/compiler/compile/render_dom/wrappers/EachBlock.ts b/src/compiler/compile/render_dom/wrappers/EachBlock.ts
@@ -149,8 +149,7 @@ export default class EachBlockWrapper extends Wrapper {
 				property: this.index_name,
 				modifier: prop.modifier,
 				snippet: prop.modifier(x`${this.vars.each_block_value}[${this.index_name}]` as Node),
-				store,
-				tail: prop.modifier(x`[${this.index_name}]` as Node)
+				store
 			});
 		});
 
@@ -347,7 +346,7 @@ 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.modifier(x`list[i]`)};`);
+		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 })};`);
 
 		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;`);
diff --git a/test/runtime/samples/each-block-destructured-default-before-initialised/_config.js b/test/runtime/samples/each-block-destructured-default-before-initialised/_config.js
@@ -0,0 +1,5 @@
+export default {
+	error(assert, err) {
+		assert.ok(err.message === "Cannot access 'c' before initialization" || err.message === 'c is not defined');
+	}
+};
diff --git a/test/runtime/samples/each-block-destructured-default-before-initialised/main.svelte b/test/runtime/samples/each-block-destructured-default-before-initialised/main.svelte
@@ -0,0 +1,7 @@
+<script>
+	let array = [{a: 1, c: 2}];
+</script>
+
+{#each array as { a, b = c, c }}
+	{a}{b}{c}
+{/each}
diff --git a/test/runtime/samples/each-block-destructured-default-binding/_config.js b/test/runtime/samples/each-block-destructured-default-binding/_config.js
@@ -0,0 +1,23 @@
+export default {
+	html: `
+		<input />
+		<input />
+	`,
+	ssrHtml: `
+		<input />
+		<input value="hello" />
+	`,
+
+	test({ assert, component, target, window }) {
+		const [input1, input2] = target.querySelectorAll('input');
+		assert.equal(input1.value, '');
+		assert.equal(input2.value, 'hello');
+
+		const inputEvent = new window.InputEvent('input');
+
+		input2.value = 'world';
+		input2.dispatchEvent(inputEvent);
+		assert.equal(input2.value, 'world');
+		assert.equal(component.array[1].value, 'world');
+	}
+};
diff --git a/test/runtime/samples/each-block-destructured-default-binding/main.svelte b/test/runtime/samples/each-block-destructured-default-binding/main.svelte
@@ -0,0 +1,7 @@
+<script>
+	export let array = [{ value: '' }, {}];
+</script>
+
+{#each array as { value = "hello" }}
+	<input bind:value />
+{/each}
diff --git a/test/runtime/samples/each-block-destructured-default/_config.js b/test/runtime/samples/each-block-destructured-default/_config.js
@@ -1,22 +1,26 @@
 export default {
 	props: {
 		animalEntries: [
-			{ animal: 'raccoon', class: 'mammal', species: 'P. lotor', kilogram: 25 },
-			{ animal: 'eagle', class: 'bird', kilogram: 5.4 }
+			{ animal: 'raccoon', class: 'mammal', species: 'P. lotor', kilogram: 25, bmi: 0.04 },
+			{ animal: 'eagle', class: 'bird', kilogram: 5.4 },
+			{ animal: 'tiger', class: 'mammal', kilogram: 10, pound: 30 },
+			{ animal: 'lion', class: 'mammal', kilogram: 10, height: 50 },
+			{ animal: 'leopard', class: 'mammal', kilogram: 30, height: 50, bmi: 10 }
 		]
 	},
 
 	html: `
-		<p class="mammal">raccoon - P. lotor - 25kg</p>
-		<p class="bird">eagle - unknown - 5.4kg</p>
+		<p class="mammal">raccoon - P. lotor - 25kg (55 lb) - 30cm - 0.04</p>
+		<p class="bird">eagle - unknown - 5.4kg (12 lb) - 30cm - 0.006</p>
+		<p class="mammal">tiger - unknown - 10kg (30 lb) - 30cm - 0.011111111111111112</p>
+		<p class="mammal">lion - unknown - 10kg (22 lb) - 50cm - 0.004</p>
+		<p class="mammal">leopard - unknown - 30kg (66 lb) - 50cm - 10</p>
 	`,
 
-	
-
 	test({ assert, component, target }) {
 		component.animalEntries = [{ animal: 'cow', class: 'mammal', species: '‎B. taurus' }];
 		assert.htmlEqual(target.innerHTML, `
-			<p class="mammal">cow - ‎B. taurus - 50kg</p>
+			<p class="mammal">cow - ‎B. taurus - 50kg (110 lb) - 30cm - 0.05555555555555555</p>
 		`);
 	}
 };
diff --git a/test/runtime/samples/each-block-destructured-default/main.svelte b/test/runtime/samples/each-block-destructured-default/main.svelte
@@ -1,7 +1,8 @@
 <script>
 	export let animalEntries;
+	export const defaultHeight = 30;
 </script>
 
-{#each animalEntries as { animal, species = 'unknown', kilogram: weight = 50 , ...props } }
-	<p {...props}>{animal} - {species} - {weight}kg</p>
+{#each animalEntries as { animal, species = 'unknown', kilogram: weight = 50, pound = (weight * 2.2).toFixed(0), height = defaultHeight, bmi = weight / (height * height), ...props } }
+	<p {...props}>{animal} - {species} - {weight}kg ({pound} lb) - {height}cm - {bmi}</p>
 {/each}

Original file line number	Diff line number	Diff line change
`@@ -33,12 +33,12 @@ export default class AwaitBlock extends Node {`
`33`	`33`
`34`	`34`	`if (this.then_node) {`
`35`	`35`	`this.then_contexts = [];`
`36`		`- unpack_destructuring(this.then_contexts, info.value, node => node);`
	`36`	`+ unpack_destructuring(this.then_contexts, info.value);`
`37`	`37`	`}`
`38`	`38`
`39`	`39`	`if (this.catch_node) {`
`40`	`40`	`this.catch_contexts = [];`
`41`		`- unpack_destructuring(this.catch_contexts, info.error, node => node);`
	`41`	`+ unpack_destructuring(this.catch_contexts, info.error);`
`42`	`42`	`}`
`43`	`43`
`44`	`44`	`this.pending = new PendingBlock(component, this, scope, info.pending);`
Original file line number	Diff line number	Diff line change
`@@ -9,7 +9,6 @@ export interface Bindings {`
`9`	`9`	`property: Identifier;`
`10`	`10`	`snippet: Node;`
`11`	`11`	`store: string;`
`12`		`- tail: Node;`
`13`	`12`	`modifier: (node: Node) => Node;`
`14`	`13`	`}`
`15`	`14`
Original file line number	Diff line number	Diff line change
`@@ -87,7 +87,7 @@ class AwaitBlockBranch extends Wrapper {`
`87`	`87`	`}`
`88`	`88`
`89`	`89`	`render_destructure() {`
`90`		- const props = this.value_contexts.map(prop => b`#ctx[${this.block.renderer.context_lookup.get(prop.key.name).index}] = ${prop.modifier(x`#ctx[${this.value_index}]`)};`);
	`90`	+ const props = 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))};`);
`91`	`91`	const get_context = this.block.renderer.component.get_unique_name(`get_${this.status}_context`);
`92`	`92`	this.block.renderer.blocks.push(b`
`93`	`93`	`function ${get_context}(#ctx) {`