fix: array rest destructuring in markup (sveltejs#8555)

Fixes sveltejs#8554

---------

Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>

src/compiler/compile/compiler_warnings.ts

@@ -220,7 +220,7 @@ export default {
 	},
 	invalid_rest_eachblock_binding: (rest_element_name: string) => ({
 		code: 'invalid-rest-eachblock-binding',
-		message: `...${rest_element_name} operator will create a new object and binding propagation with original object will not work`
+		message: `The rest operator (...) will create a new object and binding '${rest_element_name}' with the original object will not work`
 	}),
 	avoid_mouse_events_on_document: {
 		code: 'avoid-mouse-events-on-document',

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

@@ -1,5 +1,5 @@
 import { x } from 'code-red';
-import { Node, Identifier, Expression, PrivateIdentifier } from 'estree';
+import { Node, Identifier, Expression, PrivateIdentifier, Pattern } from 'estree';
 import { walk } from 'estree-walker';
 import is_reference, { NodeWithPropertyDefinition } from 'is-reference';
 import { clone } from '../../../utils/clone';
@@ -30,15 +30,17 @@ export function unpack_destructuring({
 	default_modifier = (node) => node,
 	scope,
 	component,
-	context_rest_properties
+	context_rest_properties,
+	in_rest_element = false
 }: {
 	contexts: Context[];
-	node: Node;
+	node: Pattern;
 	modifier?: DestructuredVariable['modifier'];
 	default_modifier?: DestructuredVariable['default_modifier'];
 	scope: TemplateScope;
 	component: Component;
 	context_rest_properties: Map<string, Node>;
+	in_rest_element?: boolean;
 }) {
 	if (!node) return;
 
@@ -49,28 +51,26 @@ export function unpack_destructuring({
 			modifier,
 			default_modifier
 		});
-	} else if (node.type === 'RestElement') {
-		contexts.push({
-			type: 'DestructuredVariable',
-			key: node.argument as Identifier,
-			modifier,
-			default_modifier
-		});
-		context_rest_properties.set((node.argument as Identifier).name, node);
+
+		if (in_rest_element) {
+			context_rest_properties.set(node.name, node);
+		}
 	} else if (node.type === 'ArrayPattern') {
-		node.elements.forEach((element, i) => {
-			if (element && element.type === 'RestElement') {
+		node.elements.forEach((element: Pattern | null, i: number) => {
+			if (!element) {
+				return;
+			} else if (element.type === 'RestElement') {
 				unpack_destructuring({
 					contexts,
-					node: element,
+					node: element.argument,
 					modifier: (node) => x`${modifier(node)}.slice(${i})` as Node,
 					default_modifier,
 					scope,
 					component,
-					context_rest_properties
+					context_rest_properties,
+					in_rest_element: true
 				});
-				context_rest_properties.set((element.argument as Identifier).name, element);
-			} else if (element && element.type === 'AssignmentPattern') {
+			} else if (element.type === 'AssignmentPattern') {
 				const n = contexts.length;
 				mark_referenced(element.right, scope, component);
 
@@ -87,7 +87,8 @@ export function unpack_destructuring({
 						)}` as Node,
 					scope,
 					component,
-					context_rest_properties
+					context_rest_properties,
+					in_rest_element
 				});
 			} else {
 				unpack_destructuring({
@@ -97,7 +98,8 @@ export function unpack_destructuring({
 					default_modifier,
 					scope,
 					component,
-					context_rest_properties
+					context_rest_properties,
+					in_rest_element
 				});
 			}
 		});
@@ -116,9 +118,9 @@ export function unpack_destructuring({
 					default_modifier,
 					scope,
 					component,
-					context_rest_properties
+					context_rest_properties,
+					in_rest_element: true
 				});
-				context_rest_properties.set((property.argument as Identifier).name, property);
 			} else if (property.type === 'Property') {
 				const key = property.key;
 				const value = property.value;
@@ -168,7 +170,8 @@ export function unpack_destructuring({
 							)}` as Node,
 						scope,
 						component,
-						context_rest_properties
+						context_rest_properties,
+						in_rest_element
 					});
 				} else {
 					// e.g. { property } or { property: newName }
@@ -179,7 +182,8 @@ export function unpack_destructuring({
 						default_modifier,
 						scope,
 						component,
-						context_rest_properties
+						context_rest_properties,
+						in_rest_element
 					});
 				}
 			}

test/runtime/samples/await-then-destruct-array-nested-rest/_config.js

@@ -0,0 +1,69 @@
+export default {
+	props: {
+		thePromise: new Promise(_ => {})
+	},
+
+	html: `
+		loading...
+	`,
+
+	async test({ assert, component, target }) {
+		await (component.thePromise = Promise.resolve([1, 2, 3, 4, 5, 6, 7, 8]));
+
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+				<p>a: 1</p>
+				<p>b: 2</p>
+				<p>c: 5</p>
+				<p>remaining length: 3</p>
+			`
+		);
+
+		await (component.thePromise = Promise.resolve([9, 10, 11, 12, 13, 14, 15]));
+
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+				<p>a: 9</p>
+				<p>b: 10</p>
+				<p>c: 13</p>
+				<p>remaining length: 2</p>
+			`
+		);
+
+		try {
+			await (component.thePromise = Promise.reject([16, 17, 18, 19, 20, 21, 22]));
+		} catch (e) {
+			// do nothing
+		}
+
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+				<p>c: 16</p>
+				<p>d: 17</p>
+				<p>e: 18</p>
+				<p>f: 19</p>
+				<p>g: 22</p>
+			`
+		);
+
+		try {
+			await (component.thePromise = Promise.reject([23, 24, 25, 26, 27, 28, 29, 30, 31]));
+		} catch (e) {
+			// do nothing
+		}
+
+		assert.htmlEqual(
+			target.innerHTML,
+			`
+				<p>c: 23</p>
+				<p>d: 24</p>
+				<p>e: 25</p>
+				<p>f: 26</p>
+				<p>g: 29</p>
+			`
+		);
+	}
+};

test/runtime/samples/await-then-destruct-array-nested-rest/main.svelte

@@ -0,0 +1,18 @@
+<script>
+	export let thePromise;
+</script>
+
+{#await thePromise}
+	loading...
+{:then [ a, b, ...[,, c, ...{ length } ]]}
+	<p>a: {a}</p>
+	<p>b: {b}</p>
+  <p>c: {c}</p>
+  <p>remaining length: {length}</p>
+{:catch [c, ...[d, e, f, ...[,,g]]]}
+	<p>c: {c}</p>
+	<p>d: {d}</p>
+	<p>e: {e}</p>
+  <p>f: {f}</p>
+  <p>g: {g}</p>
+{/await}

test/runtime/samples/const-tag-await-then-destructuring-nested-rest/_config.js

@@ -0,0 +1,19 @@
+export default {
+	html: '<div>12 120 70, 30+4=34</div>',
+	async test({ component, target, assert }) {
+		component.promise1 = Promise.resolve({width: 5, height: 6});
+		component.promise2 = Promise.reject({width: 6, height: 7});
+
+		await Promise.resolve();
+		assert.htmlEqual(target.innerHTML, `
+			<div>30 300 110, 50+6=56</div>
+			<div>42 420 130, 60+7=67</div>
+		`);
+
+		component.constant = 20;
+		assert.htmlEqual(target.innerHTML, `
+			<div>30 600 220, 100+6=106</div>
+			<div>42 840 260, 120+7=127</div>
+		`);
+	}
+};

test/runtime/samples/const-tag-await-then-destructuring-nested-rest/main.svelte

@@ -0,0 +1,23 @@
+<script>
+	export let promise1 = {width: 3, height: 4};
+	export let promise2 = {width: 5, height: 7};
+	export let constant = 10;
+
+	function calculate(width, height, constant) {
+		return { area: width * height, volume: width * height * constant };
+	}
+</script>
+
+{#await promise1 then { width, height }}
+	{@const {area, volume} = calculate(width, height, constant)}
+	{@const perimeter = (width + height) * constant}
+	{@const [_width, ...[_height, ...[sum]]] = [width * constant, height, width * constant + height]}
+	<div>{area} {volume} {perimeter}, {_width}+{_height}={sum}</div>
+{/await}
+
+{#await promise2 catch { width, height }}
+	{@const {area, volume} = calculate(width, height, constant)}
+	{@const perimeter = (width + height) * constant}
+	{@const [_width, ...[_height, ...[sum]]] = [width * constant, height, width * constant + height]}
+	<div>{area} {volume} {perimeter}, {_width}+{_height}={sum}</div>
+{/await}

test/runtime/samples/const-tag-each-destructure-nested-rest/_config.js

@@ -0,0 +1,30 @@
+export default {
+	html: `
+		<div>12 120 70, 30+4=34</div>
+		<div>35 350 120, 50+7=57</div>
+		<div>48 480 140, 60+8=68</div>
+	`,
+	async test({ component, target, assert }) {
+		component.constant = 20;
+
+		assert.htmlEqual(target.innerHTML, `
+			<div>12 240 140, 60+4=64</div>
+			<div>35 700 240, 100+7=107</div>
+			<div>48 960 280, 120+8=128</div>
+		`);
+
+		component.boxes = [
+			{width: 3, height: 4},
+			{width: 4, height: 5},
+			{width: 5, height: 6},
+			{width: 6, height: 7}
+		];
+
+		assert.htmlEqual(target.innerHTML, `
+			<div>12 240 140, 60+4=64</div>
+			<div>20 400 180, 80+5=85</div>
+			<div>30 600 220, 100+6=106</div>
+			<div>42 840 260, 120+7=127</div>
+		`);
+	}
+};

test/runtime/samples/const-tag-each-destructure-nested-rest/main.svelte

@@ -0,0 +1,19 @@
+<script>
+	export let boxes = [
+		{width: 3, height: 4},
+		{width: 5, height: 7},
+		{width: 6, height: 8},
+	];
+	export let constant = 10;
+
+	function calculate(width, height, constant) {
+		return { area: width * height, volume: width * height * constant };
+	}
+</script>
+
+{#each boxes as { width, height }}
+	{@const {area, volume} = calculate(width, height, constant)}
+	{@const perimeter = (width + height) * constant}
+	{@const [_width, ...[_height, ...[sum]]] = [width * constant, height, width * constant + height]}
+	<div>{area} {volume} {perimeter}, {_width}+{_height}={sum}</div>
+{/each}

test/runtime/samples/each-block-destructured-array-nested-rest/_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, Second: 2, Third: 3, Elements remaining: 2</p>
+		<p>First: 6, Second: 7, Third: 8, Elements remaining: 0</p>
+		<p>First: 9, Second: 10, Third: 11, Elements remaining: 1</p>
+		<p>First: 13, Second: 14, Third: 15, Elements remaining: 7</p>
+	`,
+
+	test({ assert, component, target }) {
+		component.array = [[23, 24, 25, 26, 27, 28, 29]];
+		assert.htmlEqual( target.innerHTML, `
+			<p>First: 23, Second: 24, Third: 25, Elements remaining: 4</p>
+		`);
+	}
+};

test/runtime/samples/each-block-destructured-array-nested-rest/main.svelte

@@ -0,0 +1,9 @@
+<script>
+	export let array;
+</script>
+
+{#each array as [first, second, ...[third, ...{ length }]]}
+	<p>
+		First: {first}, Second: {second}, Third: {third}, Elements remaining: {length}
+	</p>
+{/each}

test/validator/samples/rest-eachblock-binding-2/warnings.json

@@ -1,8 +1,8 @@
 [
   {
     "code": "invalid-rest-eachblock-binding",
-    "message": "...rest operator will create a new object and binding propagation with original object will not work",
-    "start": { "line": 8, "column": 24 },
+    "message": "The rest operator (...) will create a new object and binding 'rest' with the original object will not work",
+    "start": { "line": 8, "column": 27 },
 		"end": { "line": 8, "column": 31 }
   }
 ]

test/validator/samples/rest-eachblock-binding-3/warnings.json

@@ -1,8 +1,8 @@
 [
   {
     "code": "invalid-rest-eachblock-binding",
-    "message": "...rest operator will create a new object and binding propagation with original object will not work",
-    "start": { "line": 5, "column": 32 },
-		"end": { "line": 5, "column": 39 }
+    "message": "The rest operator (...) will create a new object and binding 'rest' with the original object will not work",
+    "start": { "line": 5, "column": 35 },
+    "end": { "line": 5, "column": 39 }
   }
 ]

test/validator/samples/rest-eachblock-binding-nested-rest/input.svelte

@@ -0,0 +1,9 @@
+<script>
+	const a = [[1, 2, 3, 4, 5]];
+</script>
+
+{#each a as [first, second, ...[third, ...{ length }]]}
+	<p>{first}, {second}, {length}</p>
+	<input bind:value={third} />
+	<input bind:value={length} />
+{/each}