Skip to content

Commit 5d7ffdb

Browse files
tanhauhautanhauhau
authored
fix function slot props based on context (#5607)
1 parent 6fa3e91 commit 5d7ffdb

File tree

13 files changed

+202
-31
lines changed

13 files changed

+202
-31
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
## Unreleased
44

55
* Fix `$$props` and `$$restProps` when compiling to a custom element ([#5482](https://github.com/sveltejs/svelte/issues/5482))
6+
* Fix function calls in `<slot>` props that use contextual values ([#5565](https://github.com/sveltejs/svelte/issues/5565))
67
* Add `Element` and `Node` to known globals ([#5586](https://github.com/sveltejs/svelte/issues/5586))
78

89
## 3.29.4

‎src/compiler/compile/nodes/shared/Expression.ts

Lines changed: 48 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,20 +4,19 @@ import is_reference from 'is-reference';
44
import flatten_reference from '../../utils/flatten_reference';
55
import { create_scopes, Scope, extract_names } from '../../utils/scope';
66
import { sanitize } from '../../../utils/names';
7-
import Wrapper from '../../render_dom/wrappers/shared/Wrapper';
87
import TemplateScope from './TemplateScope';
98
import get_object from '../../utils/get_object';
109
import Block from '../../render_dom/Block';
1110
import is_dynamic from '../../render_dom/wrappers/shared/is_dynamic';
1211
import { b } from 'code-red';
1312
import { invalidate } from '../../render_dom/invalidate';
1413
import { Node, FunctionExpression, Identifier } from 'estree';
15-
import { TemplateNode } from '../../../interfaces';
14+
import { INode } from '../interfaces';
1615
import { is_reserved_keyword } from '../../utils/reserved_keywords';
1716
import replace_object from '../../utils/replace_object';
1817
import EachBlock from '../EachBlock';
1918

20-
type Owner = Wrapper | TemplateNode;
19+
type Owner = INode;
2120

2221
export default class Expression {
2322
type: 'Expression' = 'Expression';
@@ -37,7 +36,6 @@ export default class Expression {
3736

3837
manipulated: Node;
3938

40-
// todo: owner type
4139
constructor(component: Component, owner: Owner, template_scope: TemplateScope, info, lazy?: boolean) {
4240
// TODO revert to direct property access in prod?
4341
Object.defineProperties(this, {
@@ -276,10 +274,12 @@ export default class Expression {
276274
else {
277275
// we need a combo block/init recipe
278276
const deps = Array.from(contextual_dependencies);
277+
const function_expression = node as FunctionExpression;
279278

280-
(node as FunctionExpression).params = [
279+
const has_args = function_expression.params.length > 0;
280+
function_expression.params = [
281281
...deps.map(name => ({ type: 'Identifier', name } as Identifier)),
282-
...(node as FunctionExpression).params
282+
...function_expression.params
283283
];
284284

285285
const context_args = deps.map(name => block.renderer.reference(name));
@@ -291,18 +291,49 @@ export default class Expression {
291291

292292
this.replace(id as any);
293293

294-
if ((node as FunctionExpression).params.length > 0) {
295-
declarations.push(b`
296-
function ${id}(...args) {
297-
return ${callee}(${context_args}, ...args);
298-
}
299-
`);
294+
const func_declaration = has_args
295+
? b`function ${id}(...args) {
296+
return ${callee}(${context_args}, ...args);
297+
}`
298+
: b`function ${id}() {
299+
return ${callee}(${context_args});
300+
}`;
301+
302+
if (owner.type === 'Attribute' && owner.parent.name === 'slot') {
303+
const dep_scopes = new Set<INode>(deps.map(name => template_scope.get_owner(name)));
304+
// find the nearest scopes
305+
let node: INode = owner.parent;
306+
while (node && !dep_scopes.has(node)) {
307+
node = node.parent;
308+
}
309+
310+
const func_expression = func_declaration[0];
311+
312+
if (node.type === 'InlineComponent') {
313+
// <Comp let:data />
314+
this.replace(func_expression);
315+
} else {
316+
// {#each}, {#await}
317+
const func_id = component.get_unique_name(id.name + '_func');
318+
block.renderer.add_to_context(func_id.name, true);
319+
// rename #ctx -> child_ctx;
320+
walk(func_expression, {
321+
enter(node) {
322+
if (node.type === 'Identifier' && node.name === '#ctx') {
323+
node.name = 'child_ctx';
324+
}
325+
}
326+
});
327+
// add to get_xxx_context
328+
// child_ctx[x] = function () { ... }
329+
(template_scope.get_owner(deps[0]) as EachBlock).contexts.push({
330+
key: func_id,
331+
modifier: () => func_expression
332+
});
333+
this.replace(block.renderer.reference(func_id));
334+
}
300335
} else {
301-
declarations.push(b`
302-
function ${id}() {
303-
return ${callee}(${context_args});
304-
}
305-
`);
336+
declarations.push(func_declaration);
306337
}
307338
}
308339

‎src/compiler/compile/render_dom/wrappers/EachBlock.ts

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -196,27 +196,13 @@ export default class EachBlockWrapper extends Wrapper {
196196
? !this.next.is_dom_node() :
197197
!parent_node || !this.parent.is_dom_node();
198198

199-
this.context_props = this.node.contexts.map(prop => b`child_ctx[${renderer.context_lookup.get(prop.key.name).index}] = ${prop.modifier(x`list[i]`)};`);
200-
201-
if (this.node.has_binding) this.context_props.push(b`child_ctx[${renderer.context_lookup.get(this.vars.each_block_value.name).index}] = list;`);
202-
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;`);
203-
204199
const snippet = this.node.expression.manipulate(block);
205200

206201
block.chunks.init.push(b`let ${this.vars.each_block_value} = ${snippet};`);
207202
if (this.renderer.options.dev) {
208203
block.chunks.init.push(b`@validate_each_argument(${this.vars.each_block_value});`);
209204
}
210205

211-
// TODO which is better — Object.create(array) or array.slice()?
212-
renderer.blocks.push(b`
213-
function ${this.vars.get_each_context}(#ctx, list, i) {
214-
const child_ctx = #ctx.slice();
215-
${this.context_props}
216-
return child_ctx;
217-
}
218-
`);
219-
220206
const initial_anchor_node: Identifier = { type: 'Identifier', name: parent_node ? 'null' : '#anchor' };
221207
const initial_mount_node: Identifier = parent_node || { type: 'Identifier', name: '#target' };
222208
const update_anchor_node = needs_anchor
@@ -360,6 +346,19 @@ export default class EachBlockWrapper extends Wrapper {
360346
if (this.else) {
361347
this.else.fragment.render(this.else.block, null, x`#nodes` as Identifier);
362348
}
349+
350+
this.context_props = this.node.contexts.map(prop => b`child_ctx[${renderer.context_lookup.get(prop.key.name).index}] = ${prop.modifier(x`list[i]`)};`);
351+
352+
if (this.node.has_binding) this.context_props.push(b`child_ctx[${renderer.context_lookup.get(this.vars.each_block_value.name).index}] = list;`);
353+
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;`);
354+
// TODO which is better — Object.create(array) or array.slice()?
355+
renderer.blocks.push(b`
356+
function ${this.vars.get_each_context}(#ctx, list, i) {
357+
const child_ctx = #ctx.slice();
358+
${this.context_props}
359+
return child_ctx;
360+
}
361+
`);
363362
}
364363

365364
render_keyed({

‎test/runtime/samples/component-slot-context-props-each-nested/Nested.svelte

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<script>
2+
let keys = ['a', 'b'];
3+
let items = ['c', 'd'];
4+
export let log;
5+
function setKey(key, value, item) {
6+
log.push(`setKey(${key}, ${value}, ${item})`);
7+
}
8+
</script>
9+
10+
{#each items as item (item)}
11+
{#each keys as key (key)}
12+
<slot {key} {item} set={(value) => setKey(key, value, item)} />
13+
{/each}
14+
{/each}

‎test/runtime/samples/component-slot-context-props-each-nested/_config.js

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
export default {
2+
html: `
3+
<button type="button">Set a-c</button>
4+
<button type="button">Set b-c</button>
5+
<button type="button">Set a-d</button>
6+
<button type="button">Set b-d</button>
7+
`,
8+
async test({ assert, target, window, component }) {
9+
const [btn1, btn2, btn3, btn4] = target.querySelectorAll('button');
10+
const click = new window.MouseEvent('click');
11+
12+
await btn1.dispatchEvent(click);
13+
assert.deepEqual(component.log, ['setKey(a, value-a-c, c)']);
14+
15+
await btn2.dispatchEvent(click);
16+
assert.deepEqual(component.log, [
17+
'setKey(a, value-a-c, c)',
18+
'setKey(b, value-b-c, c)'
19+
]);
20+
21+
await btn3.dispatchEvent(click);
22+
assert.deepEqual(component.log, [
23+
'setKey(a, value-a-c, c)',
24+
'setKey(b, value-b-c, c)',
25+
'setKey(a, value-a-d, d)'
26+
]);
27+
28+
await btn4.dispatchEvent(click);
29+
assert.deepEqual(component.log, [
30+
'setKey(a, value-a-c, c)',
31+
'setKey(b, value-b-c, c)',
32+
'setKey(a, value-a-d, d)',
33+
'setKey(b, value-b-d, d)'
34+
]);
35+
}
36+
};

‎test/runtime/samples/component-slot-context-props-each-nested/main.svelte

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<script>
2+
import Nested from './Nested.svelte';
3+
export let log = [];
4+
</script>
5+
6+
<Nested {log} let:set let:key let:item>
7+
<button type="button" on:click={() => set(`value-${key}-${item}`)}>Set {key}-{item}</button>
8+
</Nested>

‎test/runtime/samples/component-slot-context-props-each/Nested.svelte

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<script>
2+
let keys = ['a', 'b'];
3+
export let log;
4+
function setKey(key, value) {
5+
log.push(`setKey(${key}, ${value})`);
6+
}
7+
</script>
8+
9+
{#each keys as key (key)}
10+
<slot {key} set={(value) => setKey(key, value)} />
11+
{/each}

‎test/runtime/samples/component-slot-context-props-each/_config.js

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
export default {
2+
html: `
3+
<button type="button">Set a</button>
4+
<button type="button">Set b</button>
5+
`,
6+
async test({ assert, target, window, component }) {
7+
const [btn1, btn2] = target.querySelectorAll('button');
8+
const click = new window.MouseEvent('click');
9+
10+
await btn1.dispatchEvent(click);
11+
assert.deepEqual(component.log, ['setKey(a, value-a)']);
12+
13+
await btn2.dispatchEvent(click);
14+
assert.deepEqual(component.log, [
15+
'setKey(a, value-a)',
16+
'setKey(b, value-b)'
17+
]);
18+
}
19+
};

‎test/runtime/samples/component-slot-context-props-each/main.svelte

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<script>
2+
import Nested from './Nested.svelte';
3+
export let log = [];
4+
</script>
5+
6+
<Nested {log} let:set let:key>
7+
<button type="button" on:click={() => set(`value-${key}`)}>Set {key}</button>
8+
</Nested>

‎test/runtime/samples/component-slot-context-props-let/Inner.svelte

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<script>
2+
export let log;
3+
function setKey(key, value) {
4+
log.push(`setKey(${key}, ${value})`);
5+
}
6+
</script>
7+
8+
<slot key="a" set={setKey} />
9+
<slot key="b" set={setKey} />

0 commit comments

Comments
 (0)