msb-public
/
svelte
mirror of https://github.com/sveltejs/svelte
1
0
Code Issues Projects Releases Wiki Activity

fix: correctly determine binding scope of `let:` directives (#10395)

Browse Source 
Fixes #9797
Also added missing validation error around duplicate slots

---------

Co-authored-by: Simon Holthausen <simon.holthausen@vercel.com>
pull/10412/head
Daniel Imfeld 2 years ago committed by GitHub
parent 34019b3ba2
commit 255693e78f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 172 additions and 37 deletions
Show Stats Download Patch File Download Diff File

5
.changeset/swift-feet-juggle.md
Unescape View File

@ -0,0 +1,5 @@
---
"svelte": patch
---
fix: correctly determine binding scope of `let:` directives

4
packages/svelte/src/compiler/phases/2-analyze/validation.js
Unescape View File

@ -37,6 +37,10 @@ function validate_component(node, context) {
) {
error(attribute, 'invalid-event-modifier');
}
if (attribute.type === 'Attribute' && attribute.name === 'slot') {
validate_slot_attribute(context, attribute);
}
}
context.next({

23
packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js
Unescape View File

@ -749,7 +749,7 @@ function serialize_inline_component(node, component_name, context) {
const props_and_spreads = [];
/** @type {import('estree').ExpressionStatement[]} */
const default_lets = [];
const lets = [];
/** @type {Record<string, import('#compiler').TemplateNode[]>} */
const children = {};
@ -763,6 +763,12 @@ function serialize_inline_component(node, component_name, context) {
/** @type {import('estree').Identifier | import('estree').MemberExpression | null} */
let bind_this = null;
/**
* If this component has a slot property, it is a named slot within another component. In this case
* the slot scope applies to the component itself, too, and not just its children.
*/
let slot_scope_applies_to_itself = false;
/**
* @param {import('estree').Property} prop
*/
@ -775,12 +781,9 @@ function serialize_inline_component(node, component_name, context) {
props_and_spreads.push(props);
}
}
for (const attribute of node.attributes) {
if (attribute.type === 'LetDirective') {
default_lets.push(
/** @type {import('estree').ExpressionStatement} */ (context.visit(attribute))
);
lets.push(/** @type {import('estree').ExpressionStatement} */ (context.visit(attribute)));
} else if (attribute.type === 'OnDirective') {
events[attribute.name] ||= [];
let handler = serialize_event_handler(attribute, context);
@ -811,6 +814,10 @@ function serialize_inline_component(node, component_name, context) {
continue;
}
if (attribute.name === 'slot') {
slot_scope_applies_to_itself = true;
}
const [, value] = serialize_attribute_value(attribute.value, context);
if (attribute.metadata.dynamic) {
@ -863,6 +870,10 @@ function serialize_inline_component(node, component_name, context) {
}
}
if (slot_scope_applies_to_itself) {
context.state.init.push(...lets);
}
if (Object.keys(events).length > 0) {
const events_expression = b.object(
Object.keys(events).map((name) =>
@ -918,7 +929,7 @@ function serialize_inline_component(node, component_name, context) {
const slot_fn = b.arrow(
[b.id('$$anchor'), b.id('$$slotProps')],
b.block([...(slot_name === 'default' ? default_lets : []), ...body])
b.block([...(slot_name === 'default' && !slot_scope_applies_to_itself ? lets : []), ...body])
);
if (slot_name === 'default') {

23
packages/svelte/src/compiler/phases/3-transform/server/transform-server.js
Unescape View File

@ -808,11 +808,17 @@ function serialize_inline_component(node, component_name, context) {
const custom_css_props = [];
/** @type {import('estree').ExpressionStatement[]} */
const default_lets = [];
const lets = [];
/** @type {Record<string, import('#compiler').TemplateNode[]>} */
const children = {};
/**
* If this component has a slot property, it is a named slot within another component. In this case
* the slot scope applies to the component itself, too, and not just its children.
*/
let slot_scope_applies_to_itself = false;
/**
* @param {import('estree').Property} prop
*/
@ -825,12 +831,9 @@ function serialize_inline_component(node, component_name, context) {
props_and_spreads.push(props);
}
}
for (const attribute of node.attributes) {
if (attribute.type === 'LetDirective') {
default_lets.push(
/** @type {import('estree').ExpressionStatement} */ (context.visit(attribute))
);
lets.push(/** @type {import('estree').ExpressionStatement} */ (context.visit(attribute)));
} else if (attribute.type === 'SpreadAttribute') {
props_and_spreads.push(/** @type {import('estree').Expression} */ (context.visit(attribute)));
} else if (attribute.type === 'Attribute') {
@ -840,6 +843,10 @@ function serialize_inline_component(node, component_name, context) {
continue;
}
if (attribute.name === 'slot') {
slot_scope_applies_to_itself = true;
}
const value = serialize_attribute_value(attribute.value, context, false, true);
push_prop(b.prop('init', b.key(attribute.name), value));
} else if (attribute.type === 'BindDirective') {
@ -862,6 +869,10 @@ function serialize_inline_component(node, component_name, context) {
}
}
if (slot_scope_applies_to_itself) {
context.state.init.push(...lets);
}
/** @type {import('estree').Statement[]} */
const snippet_declarations = [];
@ -907,7 +918,7 @@ function serialize_inline_component(node, component_name, context) {
const slot_fn = b.arrow(
[b.id('$$payload'), b.id('$$slotProps')],
b.block([...(slot_name === 'default' ? default_lets : []), ...body])
b.block([...(slot_name === 'default' && !slot_scope_applies_to_itself ? lets : []), ...body])
);
if (slot_name === 'default') {

58
packages/svelte/src/compiler/phases/scope.js
Unescape View File

@ -282,7 +282,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
* @type {import('zimmerframe').Visitor<import('#compiler').ElementLike, State, import('#compiler').SvelteNode>}
*/
const SvelteFragment = (node, { state, next }) => {
const scope = analyze_let_directives(node, state.scope);
const [scope] = analyze_let_directives(node, state.scope);
scopes.set(node, scope);
next({ scope });
};
@ -293,36 +293,40 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
*/
function analyze_let_directives(node, parent) {
const scope = parent.child();
let is_default_slot = true;
for (const attribute of node.attributes) {
if (attribute.type !== 'LetDirective') continue;
/** @type {import('#compiler').Binding[]} */
const bindings = [];
scope.declarators.set(attribute, bindings);
if (attribute.type === 'LetDirective') {
/** @type {import('#compiler').Binding[]} */
const bindings = [];
scope.declarators.set(attribute, bindings);
// attach the scope to the directive itself, as well as the
// contents to which it applies
scopes.set(attribute, scope);
// attach the scope to the directive itself, as well as the
// contents to which it applies
scopes.set(attribute, scope);
if (attribute.expression) {
for (const id of extract_identifiers_from_destructuring(attribute.expression)) {
if (attribute.expression) {
for (const id of extract_identifiers_from_destructuring(attribute.expression)) {
const binding = scope.declare(id, 'derived', 'const');
bindings.push(binding);
}
} else {
/** @type {import('estree').Identifier} */
const id = {
name: attribute.name,
type: 'Identifier',
start: attribute.start,
end: attribute.end
};
const binding = scope.declare(id, 'derived', 'const');
bindings.push(binding);
}
} else {
/** @type {import('estree').Identifier} */
const id = {
name: attribute.name,
type: 'Identifier',
start: attribute.start,
end: attribute.end
};
const binding = scope.declare(id, 'derived', 'const');
bindings.push(binding);
} else if (attribute.type === 'Attribute' && attribute.name === 'slot') {
is_default_slot = false;
}
}
return scope;
return /** @type {const} */ ([scope, is_default_slot]);
}
walk(ast, state, {
@ -364,13 +368,17 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
Component(node, { state, visit, path }) {
state.scope.reference(b.id(node.name), path);
// let:x from the default slot is a weird one:
// Its scope only applies to children that are not slots themselves.
for (const attribute of node.attributes) {
visit(attribute);
}
const scope = analyze_let_directives(node, state.scope);
// let:x is super weird:
// - for the default slot, its scope only applies to children that are not slots themselves
// - for named slots, its scope applies to the component itself, too
const [scope, is_default_slot] = analyze_let_directives(node, state.scope);
if (!is_default_slot) {
scopes.set(node, scope);
}
for (const child of node.fragment.nodes) {
if (

8
packages/svelte/tests/compiler-errors/samples/component-slot-duplicate-error-5/_config.js
Unescape View File

@ -0,0 +1,8 @@
import { test } from '../../test';
export default test({
error: {
code: 'duplicate-slot-name',
message: "Duplicate slot name 'foo' in <Nested>"
}
});

9
packages/svelte/tests/compiler-errors/samples/component-slot-duplicate-error-5/main.svelte
Unescape View File

@ -0,0 +1,9 @@
<script>
import Nested from './irrelevant';
import Inner from './irrelevant';
</script>
<Nested>
<Inner slot="foo">{value}</Inner>
<Inner slot="foo">{value}</Inner>
</Nested>

8
packages/svelte/tests/compiler-errors/samples/component-slot-duplicate-error-6/_config.js
Unescape View File

@ -0,0 +1,8 @@
import { test } from '../../test';
export default test({
error: {
code: 'duplicate-slot-name',
message: "Duplicate slot name 'foo' in <Nested>"
}
});

9
packages/svelte/tests/compiler-errors/samples/component-slot-duplicate-error-6/main.svelte
Unescape View File

@ -0,0 +1,9 @@
<script>
import Nested from './irrelevant';
import Inner from './irrelevant';
</script>
<Nested>
<p slot="foo">{value}</p>
<Inner slot="foo">{value}</Inner>
</Nested>

9
packages/svelte/tests/runtime-legacy/samples/component-slot-let-named-2/Nested.svelte
Unescape View File

@ -0,0 +1,9 @@
<script>
export let things;
</script>
<div>
{#each things as thing}
<slot name="foo" {thing}/>
{/each}
</div>

5
packages/svelte/tests/runtime-legacy/samples/component-slot-let-named-2/SlotInner.svelte
Unescape View File

@ -0,0 +1,5 @@
<script>
export let thing;
</script>
<span>{thing}</span>
<slot />

36
packages/svelte/tests/runtime-legacy/samples/component-slot-let-named-2/_config.js
Unescape View File

@ -0,0 +1,36 @@
import { test } from '../../test';
export default test({
get props() {
return { things: [1, 2, 3] };
},
html: `
<div>
<span>1</span>
<div class="inner-slot">1</div>
<span>2</span>
<div class="inner-slot">2</div>
<span>3</span>
<div class="inner-slot">3</div>
</div>`,
test({ assert, component, target }) {
component.things = [1, 2, 3, 4];
assert.htmlEqual(
target.innerHTML,
`
<div>
<span>1</span>
<div class="inner-slot">1</div>
<span>2</span>
<div class="inner-slot">2</div>
<span>3</span>
<div class="inner-slot">3</div>
<span>4</span>
<div class="inner-slot">4</div>
</div>
`
);
}
});

12
packages/svelte/tests/runtime-legacy/samples/component-slot-let-named-2/main.svelte
Unescape View File

@ -0,0 +1,12 @@
<script>
import Nested from './Nested.svelte';
import SlotInner from './SlotInner.svelte';
export let things;
</script>
<Nested {things}>
<SlotInner slot="foo" let:thing={data} thing={data}>
<div class="inner-slot">{data}</div>
</SlotInner>
</Nested>
Write Preview
Loading…
Cancel
Save