From d17a90cc441edd8164530c4a33ae263196db2460 Mon Sep 17 00:00:00 2001 From: Tan Li Hau Date: Fri, 26 Feb 2021 00:36:44 +0800 Subject: [PATCH] allow destructured defaults to refer to variables (#5986) Co-authored-by: M. Habib Rosyad Co-authored-by: Conduitry --- CHANGELOG.md | 1 + src/compiler/compile/nodes/AwaitBlock.ts | 4 +- src/compiler/compile/nodes/EachBlock.ts | 2 +- src/compiler/compile/nodes/shared/Context.ts | 61 +++++++++++++++---- .../compile/nodes/shared/Expression.ts | 3 +- src/compiler/compile/render_dom/Block.ts | 1 - .../compile/render_dom/wrappers/AwaitBlock.ts | 2 +- .../compile/render_dom/wrappers/EachBlock.ts | 5 +- .../_config.js | 5 ++ .../main.svelte | 7 +++ .../_config.js | 23 +++++++ .../main.svelte | 7 +++ .../_config.js | 18 +++--- .../main.svelte | 5 +- 14 files changed, 115 insertions(+), 29 deletions(-) create mode 100644 test/runtime/samples/each-block-destructured-default-before-initialised/_config.js create mode 100644 test/runtime/samples/each-block-destructured-default-before-initialised/main.svelte create mode 100644 test/runtime/samples/each-block-destructured-default-binding/_config.js create mode 100644 test/runtime/samples/each-block-destructured-default-binding/main.svelte diff --git a/CHANGELOG.md b/CHANGELOG.md index bd57c193de..849a6e20fd 100644 --- 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 index e5b07f90be..ef74d26848 100644 --- 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 index 8968e14ab9..a0de714844 100644 --- 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 index bcc0521ffa..c6ad2c2893 100644 --- 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 index 6b051157f5..caece14f00 100644 --- 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 index c511bb0b95..63195b610f 100644 --- 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 index 2038943d8b..cc186d1c02 100644 --- 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 index 75db59161b..134976ca65 100644 --- 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 new file mode 100644 index 0000000000..c42261df40 --- /dev/null +++ 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 new file mode 100644 index 0000000000..76eb09c0de --- /dev/null +++ b/test/runtime/samples/each-block-destructured-default-before-initialised/main.svelte @@ -0,0 +1,7 @@ + + +{#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 new file mode 100644 index 0000000000..3697e37441 --- /dev/null +++ b/test/runtime/samples/each-block-destructured-default-binding/_config.js @@ -0,0 +1,23 @@ +export default { + html: ` + + + `, + ssrHtml: ` + + + `, + + 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 new file mode 100644 index 0000000000..e93a01038b --- /dev/null +++ b/test/runtime/samples/each-block-destructured-default-binding/main.svelte @@ -0,0 +1,7 @@ + + +{#each array as { value = "hello" }} + +{/each} diff --git a/test/runtime/samples/each-block-destructured-default/_config.js b/test/runtime/samples/each-block-destructured-default/_config.js index 0e99fd589f..0115c9d1ce 100644 --- 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: ` -

raccoon - P. lotor - 25kg

-

eagle - unknown - 5.4kg

+

raccoon - P. lotor - 25kg (55 lb) - 30cm - 0.04

+

eagle - unknown - 5.4kg (12 lb) - 30cm - 0.006

+

tiger - unknown - 10kg (30 lb) - 30cm - 0.011111111111111112

+

lion - unknown - 10kg (22 lb) - 50cm - 0.004

+

leopard - unknown - 30kg (66 lb) - 50cm - 10

`, - - test({ assert, component, target }) { component.animalEntries = [{ animal: 'cow', class: 'mammal', species: '‎B. taurus' }]; assert.htmlEqual(target.innerHTML, ` -

cow - ‎B. taurus - 50kg

+

cow - ‎B. taurus - 50kg (110 lb) - 30cm - 0.05555555555555555

`); } }; diff --git a/test/runtime/samples/each-block-destructured-default/main.svelte b/test/runtime/samples/each-block-destructured-default/main.svelte index a91b45299e..44e720e174 100644 --- a/test/runtime/samples/each-block-destructured-default/main.svelte +++ b/test/runtime/samples/each-block-destructured-default/main.svelte @@ -1,7 +1,8 @@ -{#each animalEntries as { animal, species = 'unknown', kilogram: weight = 50 , ...props } } -

{animal} - {species} - {weight}kg

+{#each animalEntries as { animal, species = 'unknown', kilogram: weight = 50, pound = (weight * 2.2).toFixed(0), height = defaultHeight, bmi = weight / (height * height), ...props } } +

{animal} - {species} - {weight}kg ({pound} lb) - {height}cm - {bmi}

{/each}