Skip to content

Commit

Permalink
fix: object destructuring picks up computed properties (#8386)
Browse files Browse the repository at this point in the history
fixes #6609. Prior related PR: #8357
  • Loading branch information
ngtr6788 authored Mar 15, 2023
1 parent 4b0b471 commit a1e8421
Show file tree
Hide file tree
Showing 19 changed files with 330 additions and 34 deletions.
1 change: 1 addition & 0 deletions src/compiler/compile/nodes/CatchBlock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
}
Expand Down
1 change: 1 addition & 0 deletions src/compiler/compile/nodes/ConstTag.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
1 change: 1 addition & 0 deletions src/compiler/compile/nodes/EachBlock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});

Expand Down
1 change: 1 addition & 0 deletions src/compiler/compile/nodes/ThenBlock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
}
Expand Down
69 changes: 52 additions & 17 deletions src/compiler/compile/nodes/shared/Context.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,22 @@
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';
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;
Expand All @@ -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
Expand All @@ -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') {
Expand All @@ -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({
Expand All @@ -86,7 +104,8 @@ export function unpack_destructuring({
default_modifier,
scope,
component,
context_rest_properties
context_rest_properties,
number_of_computed_props
});
}
});
Expand All @@ -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;
Expand All @@ -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 }
Expand All @@ -158,7 +190,8 @@ export function unpack_destructuring({
default_modifier,
scope,
component,
context_rest_properties
context_rest_properties,
number_of_computed_props
});
}
}
Expand All @@ -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`);
}
Expand Down
1 change: 1 addition & 0 deletions src/compiler/compile/nodes/shared/Expression.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 11 additions & 1 deletion src/compiler/compile/render_dom/wrappers/AwaitBlock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down
13 changes: 12 additions & 1 deletion src/compiler/compile/render_dom/wrappers/EachBlock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;`);
Expand Down
39 changes: 24 additions & 15 deletions src/compiler/compile/render_dom/wrappers/shared/add_const_tags.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
}
32 changes: 32 additions & 0 deletions test/runtime/samples/await-then-destruct-computed-props/_config.js
Original file line number Diff line number Diff line change
@@ -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>
`
);
}
};
Original file line number Diff line number Diff line change
@@ -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}

Loading

0 comments on commit a1e8421

Please sign in to comment.