From 5d7ffdb8a7231a772da6b259a8f2c78a4007c53b Mon Sep 17 00:00:00 2001 From: Tan Li Hau Date: Fri, 30 Oct 2020 03:15:54 +0800 Subject: [PATCH] fix function slot props based on context (#5607) --- CHANGELOG.md | 1 + .../compile/nodes/shared/Expression.ts | 65 ++++++++++++++----- .../compile/render_dom/wrappers/EachBlock.ts | 27 ++++---- .../Nested.svelte | 14 ++++ .../_config.js | 36 ++++++++++ .../main.svelte | 8 +++ .../Nested.svelte | 11 ++++ .../_config.js | 19 ++++++ .../main.svelte | 8 +++ .../Inner.svelte | 9 +++ .../Nested.svelte | 8 +++ .../_config.js | 19 ++++++ .../main.svelte | 8 +++ 13 files changed, 202 insertions(+), 31 deletions(-) create mode 100644 test/runtime/samples/component-slot-context-props-each-nested/Nested.svelte create mode 100644 test/runtime/samples/component-slot-context-props-each-nested/_config.js create mode 100644 test/runtime/samples/component-slot-context-props-each-nested/main.svelte create mode 100644 test/runtime/samples/component-slot-context-props-each/Nested.svelte create mode 100644 test/runtime/samples/component-slot-context-props-each/_config.js create mode 100644 test/runtime/samples/component-slot-context-props-each/main.svelte create mode 100644 test/runtime/samples/component-slot-context-props-let/Inner.svelte create mode 100644 test/runtime/samples/component-slot-context-props-let/Nested.svelte create mode 100644 test/runtime/samples/component-slot-context-props-let/_config.js create mode 100644 test/runtime/samples/component-slot-context-props-let/main.svelte diff --git a/CHANGELOG.md b/CHANGELOG.md index cfff198368ab..4afd7f20e096 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Unreleased * Fix `$$props` and `$$restProps` when compiling to a custom element ([#5482](https://github.com/sveltejs/svelte/issues/5482)) +* Fix function calls in `` props that use contextual values ([#5565](https://github.com/sveltejs/svelte/issues/5565)) * Add `Element` and `Node` to known globals ([#5586](https://github.com/sveltejs/svelte/issues/5586)) ## 3.29.4 diff --git a/src/compiler/compile/nodes/shared/Expression.ts b/src/compiler/compile/nodes/shared/Expression.ts index 027e962e2932..ef134f43596a 100644 --- a/src/compiler/compile/nodes/shared/Expression.ts +++ b/src/compiler/compile/nodes/shared/Expression.ts @@ -4,7 +4,6 @@ import is_reference from 'is-reference'; import flatten_reference from '../../utils/flatten_reference'; import { create_scopes, Scope, extract_names } from '../../utils/scope'; import { sanitize } from '../../../utils/names'; -import Wrapper from '../../render_dom/wrappers/shared/Wrapper'; import TemplateScope from './TemplateScope'; import get_object from '../../utils/get_object'; import Block from '../../render_dom/Block'; @@ -12,12 +11,12 @@ import is_dynamic from '../../render_dom/wrappers/shared/is_dynamic'; import { b } from 'code-red'; import { invalidate } from '../../render_dom/invalidate'; import { Node, FunctionExpression, Identifier } from 'estree'; -import { TemplateNode } from '../../../interfaces'; +import { INode } from '../interfaces'; import { is_reserved_keyword } from '../../utils/reserved_keywords'; import replace_object from '../../utils/replace_object'; import EachBlock from '../EachBlock'; -type Owner = Wrapper | TemplateNode; +type Owner = INode; export default class Expression { type: 'Expression' = 'Expression'; @@ -37,7 +36,6 @@ export default class Expression { manipulated: Node; - // todo: owner type constructor(component: Component, owner: Owner, template_scope: TemplateScope, info, lazy?: boolean) { // TODO revert to direct property access in prod? Object.defineProperties(this, { @@ -276,10 +274,12 @@ export default class Expression { else { // we need a combo block/init recipe const deps = Array.from(contextual_dependencies); + const function_expression = node as FunctionExpression; - (node as FunctionExpression).params = [ + const has_args = function_expression.params.length > 0; + function_expression.params = [ ...deps.map(name => ({ type: 'Identifier', name } as Identifier)), - ...(node as FunctionExpression).params + ...function_expression.params ]; const context_args = deps.map(name => block.renderer.reference(name)); @@ -291,18 +291,49 @@ export default class Expression { this.replace(id as any); - if ((node as FunctionExpression).params.length > 0) { - declarations.push(b` - function ${id}(...args) { - return ${callee}(${context_args}, ...args); - } - `); + const func_declaration = has_args + ? b`function ${id}(...args) { + return ${callee}(${context_args}, ...args); + }` + : b`function ${id}() { + return ${callee}(${context_args}); + }`; + + if (owner.type === 'Attribute' && owner.parent.name === 'slot') { + const dep_scopes = new Set(deps.map(name => template_scope.get_owner(name))); + // find the nearest scopes + let node: INode = owner.parent; + while (node && !dep_scopes.has(node)) { + node = node.parent; + } + + const func_expression = func_declaration[0]; + + if (node.type === 'InlineComponent') { + // + this.replace(func_expression); + } else { + // {#each}, {#await} + const func_id = component.get_unique_name(id.name + '_func'); + block.renderer.add_to_context(func_id.name, true); + // rename #ctx -> child_ctx; + walk(func_expression, { + enter(node) { + if (node.type === 'Identifier' && node.name === '#ctx') { + node.name = 'child_ctx'; + } + } + }); + // add to get_xxx_context + // child_ctx[x] = function () { ... } + (template_scope.get_owner(deps[0]) as EachBlock).contexts.push({ + key: func_id, + modifier: () => func_expression + }); + this.replace(block.renderer.reference(func_id)); + } } else { - declarations.push(b` - function ${id}() { - return ${callee}(${context_args}); - } - `); + declarations.push(func_declaration); } } diff --git a/src/compiler/compile/render_dom/wrappers/EachBlock.ts b/src/compiler/compile/render_dom/wrappers/EachBlock.ts index b31d2bcba703..a2c20e007403 100644 --- a/src/compiler/compile/render_dom/wrappers/EachBlock.ts +++ b/src/compiler/compile/render_dom/wrappers/EachBlock.ts @@ -196,11 +196,6 @@ export default class EachBlockWrapper extends Wrapper { ? !this.next.is_dom_node() : !parent_node || !this.parent.is_dom_node(); - this.context_props = this.node.contexts.map(prop => b`child_ctx[${renderer.context_lookup.get(prop.key.name).index}] = ${prop.modifier(x`list[i]`)};`); - - 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;`); - const snippet = this.node.expression.manipulate(block); block.chunks.init.push(b`let ${this.vars.each_block_value} = ${snippet};`); @@ -208,15 +203,6 @@ export default class EachBlockWrapper extends Wrapper { block.chunks.init.push(b`@validate_each_argument(${this.vars.each_block_value});`); } - // TODO which is better — Object.create(array) or array.slice()? - renderer.blocks.push(b` - function ${this.vars.get_each_context}(#ctx, list, i) { - const child_ctx = #ctx.slice(); - ${this.context_props} - return child_ctx; - } - `); - const initial_anchor_node: Identifier = { type: 'Identifier', name: parent_node ? 'null' : '#anchor' }; const initial_mount_node: Identifier = parent_node || { type: 'Identifier', name: '#target' }; const update_anchor_node = needs_anchor @@ -360,6 +346,19 @@ export default class EachBlockWrapper extends Wrapper { if (this.else) { 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]`)};`); + + 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;`); + // TODO which is better — Object.create(array) or array.slice()? + renderer.blocks.push(b` + function ${this.vars.get_each_context}(#ctx, list, i) { + const child_ctx = #ctx.slice(); + ${this.context_props} + return child_ctx; + } + `); } render_keyed({ diff --git a/test/runtime/samples/component-slot-context-props-each-nested/Nested.svelte b/test/runtime/samples/component-slot-context-props-each-nested/Nested.svelte new file mode 100644 index 000000000000..fab3ae189070 --- /dev/null +++ b/test/runtime/samples/component-slot-context-props-each-nested/Nested.svelte @@ -0,0 +1,14 @@ + + +{#each items as item (item)} + {#each keys as key (key)} + setKey(key, value, item)} /> + {/each} +{/each} \ No newline at end of file diff --git a/test/runtime/samples/component-slot-context-props-each-nested/_config.js b/test/runtime/samples/component-slot-context-props-each-nested/_config.js new file mode 100644 index 000000000000..869cc555b109 --- /dev/null +++ b/test/runtime/samples/component-slot-context-props-each-nested/_config.js @@ -0,0 +1,36 @@ +export default { + html: ` + + + + + `, + async test({ assert, target, window, component }) { + const [btn1, btn2, btn3, btn4] = target.querySelectorAll('button'); + const click = new window.MouseEvent('click'); + + await btn1.dispatchEvent(click); + assert.deepEqual(component.log, ['setKey(a, value-a-c, c)']); + + await btn2.dispatchEvent(click); + assert.deepEqual(component.log, [ + 'setKey(a, value-a-c, c)', + 'setKey(b, value-b-c, c)' + ]); + + await btn3.dispatchEvent(click); + assert.deepEqual(component.log, [ + 'setKey(a, value-a-c, c)', + 'setKey(b, value-b-c, c)', + 'setKey(a, value-a-d, d)' + ]); + + await btn4.dispatchEvent(click); + assert.deepEqual(component.log, [ + 'setKey(a, value-a-c, c)', + 'setKey(b, value-b-c, c)', + 'setKey(a, value-a-d, d)', + 'setKey(b, value-b-d, d)' + ]); + } +}; diff --git a/test/runtime/samples/component-slot-context-props-each-nested/main.svelte b/test/runtime/samples/component-slot-context-props-each-nested/main.svelte new file mode 100644 index 000000000000..7feb55aaf685 --- /dev/null +++ b/test/runtime/samples/component-slot-context-props-each-nested/main.svelte @@ -0,0 +1,8 @@ + + + + + \ No newline at end of file diff --git a/test/runtime/samples/component-slot-context-props-each/Nested.svelte b/test/runtime/samples/component-slot-context-props-each/Nested.svelte new file mode 100644 index 000000000000..a29ffe0376a6 --- /dev/null +++ b/test/runtime/samples/component-slot-context-props-each/Nested.svelte @@ -0,0 +1,11 @@ + + +{#each keys as key (key)} + setKey(key, value)} /> +{/each} \ No newline at end of file diff --git a/test/runtime/samples/component-slot-context-props-each/_config.js b/test/runtime/samples/component-slot-context-props-each/_config.js new file mode 100644 index 000000000000..f374e7f82701 --- /dev/null +++ b/test/runtime/samples/component-slot-context-props-each/_config.js @@ -0,0 +1,19 @@ +export default { + html: ` + + + `, + async test({ assert, target, window, component }) { + const [btn1, btn2] = target.querySelectorAll('button'); + const click = new window.MouseEvent('click'); + + await btn1.dispatchEvent(click); + assert.deepEqual(component.log, ['setKey(a, value-a)']); + + await btn2.dispatchEvent(click); + assert.deepEqual(component.log, [ + 'setKey(a, value-a)', + 'setKey(b, value-b)' + ]); + } +}; diff --git a/test/runtime/samples/component-slot-context-props-each/main.svelte b/test/runtime/samples/component-slot-context-props-each/main.svelte new file mode 100644 index 000000000000..7dc17b4c955a --- /dev/null +++ b/test/runtime/samples/component-slot-context-props-each/main.svelte @@ -0,0 +1,8 @@ + + + + + \ No newline at end of file diff --git a/test/runtime/samples/component-slot-context-props-let/Inner.svelte b/test/runtime/samples/component-slot-context-props-let/Inner.svelte new file mode 100644 index 000000000000..6beaabbb6888 --- /dev/null +++ b/test/runtime/samples/component-slot-context-props-let/Inner.svelte @@ -0,0 +1,9 @@ + + + + \ No newline at end of file diff --git a/test/runtime/samples/component-slot-context-props-let/Nested.svelte b/test/runtime/samples/component-slot-context-props-let/Nested.svelte new file mode 100644 index 000000000000..97c44c51454e --- /dev/null +++ b/test/runtime/samples/component-slot-context-props-let/Nested.svelte @@ -0,0 +1,8 @@ + + + + set(key, value)} /> + \ No newline at end of file diff --git a/test/runtime/samples/component-slot-context-props-let/_config.js b/test/runtime/samples/component-slot-context-props-let/_config.js new file mode 100644 index 000000000000..f374e7f82701 --- /dev/null +++ b/test/runtime/samples/component-slot-context-props-let/_config.js @@ -0,0 +1,19 @@ +export default { + html: ` + + + `, + async test({ assert, target, window, component }) { + const [btn1, btn2] = target.querySelectorAll('button'); + const click = new window.MouseEvent('click'); + + await btn1.dispatchEvent(click); + assert.deepEqual(component.log, ['setKey(a, value-a)']); + + await btn2.dispatchEvent(click); + assert.deepEqual(component.log, [ + 'setKey(a, value-a)', + 'setKey(b, value-b)' + ]); + } +}; diff --git a/test/runtime/samples/component-slot-context-props-let/main.svelte b/test/runtime/samples/component-slot-context-props-let/main.svelte new file mode 100644 index 000000000000..a061f73b3a5d --- /dev/null +++ b/test/runtime/samples/component-slot-context-props-let/main.svelte @@ -0,0 +1,8 @@ + + + + +