Skip to content

allow destructured defaults to refer to variables #5986

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Feb 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/compiler/compile/nodes/AwaitBlock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/compile/nodes/EachBlock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
61 changes: 50 additions & 11 deletions src/compiler/compile/nodes/shared/Context.ts
Original file line number Diff line number Diff line change
@@ -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') {
Expand All @@ -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;
}
3 changes: 2 additions & 1 deletion src/compiler/compile/nodes/shared/Expression.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down
1 change: 0 additions & 1 deletion src/compiler/compile/render_dom/Block.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ export interface Bindings {
property: Identifier;
snippet: Node;
store: string;
tail: Node;
modifier: (node: Node) => Node;
}

Expand Down
2 changes: 1 addition & 1 deletion src/compiler/compile/render_dom/wrappers/AwaitBlock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
5 changes: 2 additions & 3 deletions src/compiler/compile/render_dom/wrappers/EachBlock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
});
});

Expand Down Expand Up @@ -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;`);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export default {
error(assert, err) {
assert.ok(err.message === "Cannot access 'c' before initialization" || err.message === 'c is not defined');
}
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<script>
let array = [{a: 1, c: 2}];
</script>

{#each array as { a, b = c, c }}
{a}{b}{c}
{/each}
Original file line number Diff line number Diff line change
@@ -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');
}
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<script>
export let array = [{ value: '' }, {}];
</script>

{#each array as { value = "hello" }}
<input bind:value />
{/each}
18 changes: 11 additions & 7 deletions test/runtime/samples/each-block-destructured-default/_config.js
Original file line number Diff line number Diff line change
@@ -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>
`);
}
};
Original file line number Diff line number Diff line change
@@ -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}