diff --git a/src/compiler/compile/compiler_warnings.ts b/src/compiler/compile/compiler_warnings.ts index 5fc5faa8f1..b3e2e36797 100644 --- a/src/compiler/compile/compiler_warnings.ts +++ b/src/compiler/compile/compiler_warnings.ts @@ -186,5 +186,9 @@ export default { redundant_event_modifier_passive: { code: 'redundant-event-modifier', message: 'The passive modifier only works with wheel and touch events' - } + }, + 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 propogation with original object will not work` + }) }; diff --git a/src/compiler/compile/nodes/AwaitBlock.ts b/src/compiler/compile/nodes/AwaitBlock.ts index 735fdbfff3..4a669b6365 100644 --- a/src/compiler/compile/nodes/AwaitBlock.ts +++ b/src/compiler/compile/nodes/AwaitBlock.ts @@ -23,6 +23,8 @@ export default class AwaitBlock extends Node { then: ThenBlock; catch: CatchBlock; + context_rest_properties: Map = new Map(); + constructor(component: Component, parent: Node, scope: TemplateScope, info: TemplateNode) { super(component, parent, scope, info); @@ -33,12 +35,12 @@ export default class AwaitBlock extends Node { if (this.then_node) { this.then_contexts = []; - unpack_destructuring({ contexts: this.then_contexts, node: info.value, scope, component }); + unpack_destructuring({ contexts: this.then_contexts, node: info.value, scope, component, context_rest_properties: this.context_rest_properties }); } if (this.catch_node) { this.catch_contexts = []; - unpack_destructuring({ contexts: this.catch_contexts, node: info.error, scope, component }); + unpack_destructuring({ contexts: this.catch_contexts, node: info.error, scope, component, context_rest_properties: this.context_rest_properties }); } this.pending = new PendingBlock(component, this, scope, info.pending); diff --git a/src/compiler/compile/nodes/Binding.ts b/src/compiler/compile/nodes/Binding.ts index 39f6fa374e..594490a5fb 100644 --- a/src/compiler/compile/nodes/Binding.ts +++ b/src/compiler/compile/nodes/Binding.ts @@ -11,6 +11,7 @@ import InlineComponent from './InlineComponent'; import Window from './Window'; import { clone } from '../../utils/clone'; import compiler_errors from '../compiler_errors'; +import compiler_warnings from '../compiler_warnings'; // TODO this should live in a specific binding const read_only_media_attributes = new Set([ @@ -47,6 +48,7 @@ export default class Binding extends Node { const { name } = get_object(this.expression.node); this.is_contextual = Array.from(this.expression.references).some(name => scope.names.has(name)); + if (this.is_contextual) this.validate_binding_rest_properties(scope); // make sure we track this as a mutable ref if (scope.is_let(name)) { @@ -95,6 +97,18 @@ export default class Binding extends Node { is_readonly_media_attribute() { return read_only_media_attributes.has(this.name); } + + validate_binding_rest_properties(scope: TemplateScope) { + this.expression.references.forEach(name => { + const each_block = scope.get_owner(name); + if (each_block && each_block.type === 'EachBlock') { + const rest_node = each_block.context_rest_properties.get(name); + if (rest_node) { + this.component.warn(rest_node as any, compiler_warnings.invalid_rest_eachblock_binding(name)); + } + } + }); + } } function isElement(node: Node): node is Element { diff --git a/src/compiler/compile/nodes/ConstTag.ts b/src/compiler/compile/nodes/ConstTag.ts index 87a9039008..44a50aa005 100644 --- a/src/compiler/compile/nodes/ConstTag.ts +++ b/src/compiler/compile/nodes/ConstTag.ts @@ -10,6 +10,7 @@ import { extract_identifiers } from 'periscopic'; import is_reference, { NodeWithPropertyDefinition } from 'is-reference'; import get_object from '../utils/get_object'; import compiler_errors from '../compiler_errors'; +import { Node as ESTreeNode } from 'estree'; const allowed_parents = new Set(['EachBlock', 'CatchBlock', 'ThenBlock', 'InlineComponent', 'SlotTemplate', 'IfBlock', 'ElseBlock']); @@ -19,6 +20,7 @@ export default class ConstTag extends Node { contexts: Context[] = []; node: ConstTagType; scope: TemplateScope; + context_rest_properties: Map = new Map(); assignees: Set = new Set(); dependencies: Set = new Set(); @@ -58,7 +60,8 @@ export default class ConstTag extends Node { contexts: this.contexts, node: this.node.expression.left, scope: this.scope, - component: this.component + component: this.component, + context_rest_properties: this.context_rest_properties }); this.expression = new Expression(this.component, this, this.scope, this.node.expression.right); this.contexts.forEach(context => { diff --git a/src/compiler/compile/nodes/EachBlock.ts b/src/compiler/compile/nodes/EachBlock.ts index bea6fb7910..4a5ea19e37 100644 --- a/src/compiler/compile/nodes/EachBlock.ts +++ b/src/compiler/compile/nodes/EachBlock.ts @@ -28,7 +28,7 @@ export default class EachBlock extends AbstractBlock { has_animation: boolean; has_binding = false; has_index_binding = false; - + context_rest_properties: Map; else?: ElseBlock; constructor(component: Component, parent: Node, scope: TemplateScope, info: TemplateNode) { @@ -40,9 +40,9 @@ export default class EachBlock extends AbstractBlock { this.index = info.index; this.scope = scope.child(); - + this.context_rest_properties = new Map(); this.contexts = []; - unpack_destructuring({ contexts: this.contexts, node: info.context, scope, component }); + unpack_destructuring({ contexts: this.contexts, node: info.context, scope, component, context_rest_properties: this.context_rest_properties }); 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 670f0531e6..47180d24cd 100644 --- a/src/compiler/compile/nodes/shared/Context.ts +++ b/src/compiler/compile/nodes/shared/Context.ts @@ -20,7 +20,8 @@ export function unpack_destructuring({ modifier = (node) => node, default_modifier = (node) => node, scope, - component + component, + context_rest_properties }: { contexts: Context[]; node: Node; @@ -28,6 +29,7 @@ export function unpack_destructuring({ default_modifier?: Context['default_modifier']; scope: TemplateScope; component: Component; + context_rest_properties: Map; }) { if (!node) return; @@ -43,6 +45,7 @@ export function unpack_destructuring({ modifier, default_modifier }); + context_rest_properties.set((node.argument as Identifier).name, node); } else if (node.type === 'ArrayPattern') { node.elements.forEach((element, i) => { if (element && element.type === 'RestElement') { @@ -52,8 +55,10 @@ export function unpack_destructuring({ modifier: (node) => x`${modifier(node)}.slice(${i})` as Node, default_modifier, scope, - component + component, + context_rest_properties }); + context_rest_properties.set((element.argument as Identifier).name, element); } else if (element && element.type === 'AssignmentPattern') { const n = contexts.length; mark_referenced(element.right, scope, component); @@ -70,7 +75,8 @@ export function unpack_destructuring({ to_ctx )}` as Node, scope, - component + component, + context_rest_properties }); } else { unpack_destructuring({ @@ -79,7 +85,8 @@ export function unpack_destructuring({ modifier: (node) => x`${modifier(node)}[${i}]` as Node, default_modifier, scope, - component + component, + context_rest_properties }); } }); @@ -97,8 +104,10 @@ export function unpack_destructuring({ )}, [${used_properties}])` as Node, default_modifier, scope, - component + component, + context_rest_properties }); + context_rest_properties.set((property.argument as Identifier).name, property); } else { const key = property.key as Identifier; const value = property.value; @@ -121,7 +130,8 @@ export function unpack_destructuring({ to_ctx )}` as Node, scope, - component + component, + context_rest_properties }); } else { unpack_destructuring({ @@ -130,7 +140,8 @@ export function unpack_destructuring({ modifier: (node) => x`${modifier(node)}.${key.name}` as Node, default_modifier, scope, - component + component, + context_rest_properties }); } } diff --git a/test/validator/samples/rest-eachblock-binding-2/input.svelte b/test/validator/samples/rest-eachblock-binding-2/input.svelte new file mode 100644 index 0000000000..d92557673b --- /dev/null +++ b/test/validator/samples/rest-eachblock-binding-2/input.svelte @@ -0,0 +1,11 @@ + + +{#each objArray as [id, ...rest] (id)} + +
+{/each} diff --git a/test/validator/samples/rest-eachblock-binding-2/warnings.json b/test/validator/samples/rest-eachblock-binding-2/warnings.json new file mode 100644 index 0000000000..a55e08ac05 --- /dev/null +++ b/test/validator/samples/rest-eachblock-binding-2/warnings.json @@ -0,0 +1,9 @@ +[ + { + "code": "invalid-rest-eachblock-binding", + "message": "...rest operator will create a new object and binding propogation with original object will not work", + "pos": 102, + "start": { "line": 8, "column": 24, "character": 102 }, + "end": { "line": 8, "column": 31, "character": 109 } + } +] diff --git a/test/validator/samples/rest-eachblock-binding-3/input.svelte b/test/validator/samples/rest-eachblock-binding-3/input.svelte new file mode 100644 index 0000000000..b8bae0cd7f --- /dev/null +++ b/test/validator/samples/rest-eachblock-binding-3/input.svelte @@ -0,0 +1,8 @@ + + +{#each objArray as { bar: { id, ...rest } } (id)} + +
+{/each} diff --git a/test/validator/samples/rest-eachblock-binding-3/warnings.json b/test/validator/samples/rest-eachblock-binding-3/warnings.json new file mode 100644 index 0000000000..c3410b3888 --- /dev/null +++ b/test/validator/samples/rest-eachblock-binding-3/warnings.json @@ -0,0 +1,9 @@ +[ + { + "code": "invalid-rest-eachblock-binding", + "message": "...rest operator will create a new object and binding propogation with original object will not work", + "pos": 168, + "start": { "line": 5, "column": 32, "character": 168 }, + "end": { "line": 5, "column": 39, "character": 175 } + } +] diff --git a/test/validator/samples/rest-eachblock-binding/input.svelte b/test/validator/samples/rest-eachblock-binding/input.svelte new file mode 100644 index 0000000000..7b96705448 --- /dev/null +++ b/test/validator/samples/rest-eachblock-binding/input.svelte @@ -0,0 +1,8 @@ + + +{#each objArray as { id, ...rest } (id)} + +
+{/each} diff --git a/test/validator/samples/rest-eachblock-binding/warnings.json b/test/validator/samples/rest-eachblock-binding/warnings.json new file mode 100644 index 0000000000..d5d34f31d1 --- /dev/null +++ b/test/validator/samples/rest-eachblock-binding/warnings.json @@ -0,0 +1,9 @@ +[ + { + "code": "invalid-rest-eachblock-binding", + "message": "...rest operator will create a new object and binding propogation with original object will not work", + "pos": 143, + "start": { "line": 5, "column": 25, "character": 143 }, + "end": { "line": 5, "column": 32, "character": 150 } + } +]