Skip to content
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

[feat] add referenced for default values in destructuring context #7007

Merged
merged 3 commits into from
Jan 6, 2022
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
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);
unpack_destructuring({ contexts: this.then_contexts, node: info.value, scope, component });
}

if (this.catch_node) {
this.catch_contexts = [];
unpack_destructuring(this.catch_contexts, info.error);
unpack_destructuring({ contexts: this.catch_contexts, node: info.error, scope, component });
}

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 @@ -39,7 +39,7 @@ export default class EachBlock extends AbstractBlock {
this.scope = scope.child();

this.contexts = [];
unpack_destructuring(this.contexts, info.context);
unpack_destructuring({ contexts: this.contexts, node: info.context, scope, component });

this.contexts.forEach(context => {
this.scope.add(context.key.name, this.expression.dependencies, this);
Expand Down
125 changes: 112 additions & 13 deletions src/compiler/compile/nodes/shared/Context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ import { Node, Identifier, Expression } 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 {
key: Identifier;
Expand All @@ -11,7 +14,21 @@ export interface Context {
default_modifier: (node: Node, to_ctx: (name: string) => Node) => Node;
}

export function unpack_destructuring(contexts: Context[], node: Node, modifier: Context['modifier'] = node => node, default_modifier: Context['default_modifier'] = node => node) {
export function unpack_destructuring({
contexts,
node,
modifier = (node) => node,
default_modifier = (node) => node,
scope,
component
}: {
contexts: Context[];
node: Node;
modifier?: Context['modifier'];
default_modifier?: Context['default_modifier'];
scope: TemplateScope;
component: Component;
}) {
if (!node) return;

if (node.type === 'Identifier') {
Expand All @@ -29,26 +46,59 @@ export function unpack_destructuring(contexts: Context[], node: Node, 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, default_modifier);
unpack_destructuring({
contexts,
node: element,
modifier: (node) => x`${modifier(node)}.slice(${i})` as Node,
default_modifier,
scope,
component
});
} else if (element && element.type === 'AssignmentPattern') {
const n = contexts.length;
mark_referenced(element.right, scope, component);

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);
unpack_destructuring({
contexts,
node: element.left,
modifier: (node) => x`${modifier(node)}[${i}]`,
default_modifier: (node, to_ctx) =>
x`${node} !== undefined ? ${node} : ${update_reference(
contexts,
n,
element.right,
to_ctx
)}` as Node,
scope,
component
});
} else {
unpack_destructuring(contexts, element, node => x`${modifier(node)}[${i}]` as Node, default_modifier);
unpack_destructuring({
contexts,
node: element,
modifier: (node) => x`${modifier(node)}[${i}]` as Node,
default_modifier,
scope,
component
});
}
});
} else if (node.type === 'ObjectPattern') {
const used_properties = [];

node.properties.forEach((property) => {
if (property.type === 'RestElement') {
unpack_destructuring(
unpack_destructuring({
contexts,
property.argument,
node => x`@object_without_properties(${modifier(node)}, [${used_properties}])` as Node,
default_modifier
);
node: property.argument,
modifier: (node) =>
x`@object_without_properties(${modifier(
node
)}, [${used_properties}])` as Node,
default_modifier,
scope,
component
});
} else {
const key = property.key as Identifier;
const value = property.value;
Expand All @@ -57,16 +107,43 @@ export function unpack_destructuring(contexts: Context[], node: Node, modifier:
if (value.type === 'AssignmentPattern') {
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);
mark_referenced(value.right, scope, component);

unpack_destructuring({
contexts,
node: value.left,
modifier: (node) => x`${modifier(node)}.${key.name}`,
default_modifier: (node, to_ctx) =>
x`${node} !== undefined ? ${node} : ${update_reference(
contexts,
n,
value.right,
to_ctx
)}` as Node,
scope,
component
});
} else {
unpack_destructuring(contexts, value, node => x`${modifier(node)}.${key.name}` as Node, default_modifier);
unpack_destructuring({
contexts,
node: value,
modifier: (node) => x`${modifier(node)}.${key.name}` as Node,
default_modifier,
scope,
component
});
}
}
});
}
}

function update_reference(contexts: Context[], n: number, expression: Expression, to_ctx: (name: string) => Node): Node {
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];
Expand All @@ -85,7 +162,12 @@ function update_reference(contexts: Context[], n: number, expression: Expression
expression = clone(expression) as Expression;
walk(expression, {
enter(node, parent: Node) {
if (is_reference(node as NodeWithPropertyDefinition, parent as NodeWithPropertyDefinition)) {
if (
is_reference(
node as NodeWithPropertyDefinition,
parent as NodeWithPropertyDefinition
)
) {
this.replace(find_from_context(node as Identifier));
this.skip();
}
Expand All @@ -94,3 +176,20 @@ function update_reference(contexts: Context[], n: number, expression: Expression

return expression;
}

function mark_referenced(
node: Node,
scope: TemplateScope,
component: Component
) {
walk(node, {
enter(node: any, parent: any) {
if (is_reference(node, parent)) {
const { name } = flatten_reference(node);
if (!scope.is_let(name) && !scope.names.has(name)) {
component.add_reference(name);
}
}
}
});
}
20 changes: 20 additions & 0 deletions test/validator/samples/unreferenced-variables-each/input.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<script>
let Component;
export let array;
export let default_value_1;
export let default_value_2;
export let default_value_3;
export let default_value_4;
export let default_value_5;
export let default_value_6;
</script>

<Component>
<svelte:fragment let:a={default_value_6}>
{#each array as default_value_5}
{#each array as { a = default_value_1, b: { b = [default_value_2, default_value_3], c: [c = default_value_4] }, d = default_value_5, e = default_value_6 }}
{a}{b}{c}{d}{e}
{/each}
{/each}
</svelte:fragment>
</Component>
32 changes: 32 additions & 0 deletions test/validator/samples/unreferenced-variables-each/warnings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
[
{
"code": "unused-export-let",
"message": "Component_1 has unused export property 'default_value_5'. If it is for external reference only, please consider using `export const default_value_5`",
"pos": 172,
"start": {
"character": 172,
"column": 12,
"line": 8
},
"end": {
"character": 187,
"column": 27,
"line": 8
}
},
{
"code": "unused-export-let",
"message": "Component_1 has unused export property 'default_value_6'. If it is for external reference only, please consider using `export const default_value_6`",
"pos": 201,
"start": {
"character": 201,
"column": 12,
"line": 9
},
"end": {
"character": 216,
"column": 27,
"line": 9
}
}
]