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

fix function slot props based on context #5607

Merged
merged 3 commits into from
Oct 29, 2020
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<slot>` 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
Expand Down
65 changes: 48 additions & 17 deletions src/compiler/compile/nodes/shared/Expression.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,19 @@ 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';
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';
Expand All @@ -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, {
Expand Down Expand Up @@ -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));
Expand All @@ -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<INode>(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') {
// <Comp let:data />
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);
}
}

Expand Down
27 changes: 13 additions & 14 deletions src/compiler/compile/render_dom/wrappers/EachBlock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,27 +196,13 @@ 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};`);
if (this.renderer.options.dev) {
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
Expand Down Expand Up @@ -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({
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<script>
let keys = ['a', 'b'];
let items = ['c', 'd'];
export let log;
function setKey(key, value, item) {
log.push(`setKey(${key}, ${value}, ${item})`);
}
</script>

{#each items as item (item)}
{#each keys as key (key)}
<slot {key} {item} set={(value) => setKey(key, value, item)} />
{/each}
{/each}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
export default {
html: `
<button type="button">Set a-c</button>
<button type="button">Set b-c</button>
<button type="button">Set a-d</button>
<button type="button">Set b-d</button>
`,
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)'
]);
}
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<script>
import Nested from './Nested.svelte';
export let log = [];
</script>

<Nested {log} let:set let:key let:item>
<button type="button" on:click={() => set(`value-${key}-${item}`)}>Set {key}-{item}</button>
</Nested>
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<script>
let keys = ['a', 'b'];
export let log;
function setKey(key, value) {
log.push(`setKey(${key}, ${value})`);
}
</script>

{#each keys as key (key)}
<slot {key} set={(value) => setKey(key, value)} />
{/each}
19 changes: 19 additions & 0 deletions test/runtime/samples/component-slot-context-props-each/_config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
export default {
html: `
<button type="button">Set a</button>
<button type="button">Set b</button>
`,
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)'
]);
}
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<script>
import Nested from './Nested.svelte';
export let log = [];
</script>

<Nested {log} let:set let:key>
<button type="button" on:click={() => set(`value-${key}`)}>Set {key}</button>
</Nested>
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<script>
export let log;
function setKey(key, value) {
log.push(`setKey(${key}, ${value})`);
}
</script>

<slot key="a" set={setKey} />
<slot key="b" set={setKey} />
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<script>
import Inner from './Inner.svelte';
export let log;
</script>

<Inner {log} let:key let:set>
<slot {key} set={(value) => set(key, value)} />
</Inner>
19 changes: 19 additions & 0 deletions test/runtime/samples/component-slot-context-props-let/_config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
export default {
html: `
<button type="button">Set a</button>
<button type="button">Set b</button>
`,
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)'
]);
}
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<script>
import Nested from './Nested.svelte';
export let log = [];
</script>

<Nested {log} let:set let:key>
<button type="button" on:click={() => set(`value-${key}`)}>Set {key}</button>
</Nested>