Skip to content

Commit 255693e

Browse files
dimfelddummdidumm
dimfeld
and
dummdidumm
authored
fix: correctly determine binding scope of let: directives (#10395)
Fixes #9797 Also added missing validation error around duplicate slots --------- Co-authored-by: Simon Holthausen <[email protected]>
1 parent 34019b3 commit 255693e

File tree

13 files changed

+172
-37
lines changed

13 files changed

+172
-37
lines changed

‎.changeset/swift-feet-juggle.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"svelte": patch
3+
---
4+
5+
fix: correctly determine binding scope of `let:` directives

‎packages/svelte/src/compiler/phases/2-analyze/validation.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@ function validate_component(node, context) {
3737
) {
3838
error(attribute, 'invalid-event-modifier');
3939
}
40+
41+
if (attribute.type === 'Attribute' && attribute.name === 'slot') {
42+
validate_slot_attribute(context, attribute);
43+
}
4044
}
4145

4246
context.next({

‎packages/svelte/src/compiler/phases/3-transform/client/visitors/template.js‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -749,7 +749,7 @@ function serialize_inline_component(node, component_name, context) {
749749
const props_and_spreads = [];
750750

751751
/** @type {import('estree').ExpressionStatement[]} */
752-
const default_lets = [];
752+
const lets = [];
753753

754754
/** @type {Record<string, import('#compiler').TemplateNode[]>} */
755755
const children = {};
@@ -763,6 +763,12 @@ function serialize_inline_component(node, component_name, context) {
763763
/** @type {import('estree').Identifier | import('estree').MemberExpression | null} */
764764
let bind_this = null;
765765

766+
/**
767+
* If this component has a slot property, it is a named slot within another component. In this case
768+
* the slot scope applies to the component itself, too, and not just its children.
769+
*/
770+
let slot_scope_applies_to_itself = false;
771+
766772
/**
767773
* @param {import('estree').Property} prop
768774
*/
@@ -775,12 +781,9 @@ function serialize_inline_component(node, component_name, context) {
775781
props_and_spreads.push(props);
776782
}
777783
}
778-
779784
for (const attribute of node.attributes) {
780785
if (attribute.type === 'LetDirective') {
781-
default_lets.push(
782-
/** @type {import('estree').ExpressionStatement} */ (context.visit(attribute))
783-
);
786+
lets.push(/** @type {import('estree').ExpressionStatement} */ (context.visit(attribute)));
784787
} else if (attribute.type === 'OnDirective') {
785788
events[attribute.name] ||= [];
786789
let handler = serialize_event_handler(attribute, context);
@@ -811,6 +814,10 @@ function serialize_inline_component(node, component_name, context) {
811814
continue;
812815
}
813816

817+
if (attribute.name === 'slot') {
818+
slot_scope_applies_to_itself = true;
819+
}
820+
814821
const [, value] = serialize_attribute_value(attribute.value, context);
815822

816823
if (attribute.metadata.dynamic) {
@@ -863,6 +870,10 @@ function serialize_inline_component(node, component_name, context) {
863870
}
864871
}
865872

873+
if (slot_scope_applies_to_itself) {
874+
context.state.init.push(...lets);
875+
}
876+
866877
if (Object.keys(events).length > 0) {
867878
const events_expression = b.object(
868879
Object.keys(events).map((name) =>
@@ -918,7 +929,7 @@ function serialize_inline_component(node, component_name, context) {
918929

919930
const slot_fn = b.arrow(
920931
[b.id('$$anchor'), b.id('$$slotProps')],
921-
b.block([...(slot_name === 'default' ? default_lets : []), ...body])
932+
b.block([...(slot_name === 'default' && !slot_scope_applies_to_itself ? lets : []), ...body])
922933
);
923934

924935
if (slot_name === 'default') {

‎packages/svelte/src/compiler/phases/3-transform/server/transform-server.js‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -808,11 +808,17 @@ function serialize_inline_component(node, component_name, context) {
808808
const custom_css_props = [];
809809

810810
/** @type {import('estree').ExpressionStatement[]} */
811-
const default_lets = [];
811+
const lets = [];
812812

813813
/** @type {Record<string, import('#compiler').TemplateNode[]>} */
814814
const children = {};
815815

816+
/**
817+
* If this component has a slot property, it is a named slot within another component. In this case
818+
* the slot scope applies to the component itself, too, and not just its children.
819+
*/
820+
let slot_scope_applies_to_itself = false;
821+
816822
/**
817823
* @param {import('estree').Property} prop
818824
*/
@@ -825,12 +831,9 @@ function serialize_inline_component(node, component_name, context) {
825831
props_and_spreads.push(props);
826832
}
827833
}
828-
829834
for (const attribute of node.attributes) {
830835
if (attribute.type === 'LetDirective') {
831-
default_lets.push(
832-
/** @type {import('estree').ExpressionStatement} */ (context.visit(attribute))
833-
);
836+
lets.push(/** @type {import('estree').ExpressionStatement} */ (context.visit(attribute)));
834837
} else if (attribute.type === 'SpreadAttribute') {
835838
props_and_spreads.push(/** @type {import('estree').Expression} */ (context.visit(attribute)));
836839
} else if (attribute.type === 'Attribute') {
@@ -840,6 +843,10 @@ function serialize_inline_component(node, component_name, context) {
840843
continue;
841844
}
842845

846+
if (attribute.name === 'slot') {
847+
slot_scope_applies_to_itself = true;
848+
}
849+
843850
const value = serialize_attribute_value(attribute.value, context, false, true);
844851
push_prop(b.prop('init', b.key(attribute.name), value));
845852
} else if (attribute.type === 'BindDirective') {
@@ -862,6 +869,10 @@ function serialize_inline_component(node, component_name, context) {
862869
}
863870
}
864871

872+
if (slot_scope_applies_to_itself) {
873+
context.state.init.push(...lets);
874+
}
875+
865876
/** @type {import('estree').Statement[]} */
866877
const snippet_declarations = [];
867878

@@ -907,7 +918,7 @@ function serialize_inline_component(node, component_name, context) {
907918

908919
const slot_fn = b.arrow(
909920
[b.id('$$payload'), b.id('$$slotProps')],
910-
b.block([...(slot_name === 'default' ? default_lets : []), ...body])
921+
b.block([...(slot_name === 'default' && !slot_scope_applies_to_itself ? lets : []), ...body])
911922
);
912923

913924
if (slot_name === 'default') {

‎packages/svelte/src/compiler/phases/scope.js‎

Lines changed: 33 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
282282
* @type {import('zimmerframe').Visitor<import('#compiler').ElementLike, State, import('#compiler').SvelteNode>}
283283
*/
284284
const SvelteFragment = (node, { state, next }) => {
285-
const scope = analyze_let_directives(node, state.scope);
285+
const [scope] = analyze_let_directives(node, state.scope);
286286
scopes.set(node, scope);
287287
next({ scope });
288288
};
@@ -293,36 +293,40 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
293293
*/
294294
function analyze_let_directives(node, parent) {
295295
const scope = parent.child();
296+
let is_default_slot = true;
296297

297298
for (const attribute of node.attributes) {
298-
if (attribute.type !== 'LetDirective') continue;
299-
300-
/** @type {import('#compiler').Binding[]} */
301-
const bindings = [];
302-
scope.declarators.set(attribute, bindings);
299+
if (attribute.type === 'LetDirective') {
300+
/** @type {import('#compiler').Binding[]} */
301+
const bindings = [];
302+
scope.declarators.set(attribute, bindings);
303303

304-
// attach the scope to the directive itself, as well as the
305-
// contents to which it applies
306-
scopes.set(attribute, scope);
304+
// attach the scope to the directive itself, as well as the
305+
// contents to which it applies
306+
scopes.set(attribute, scope);
307307

308-
if (attribute.expression) {
309-
for (const id of extract_identifiers_from_destructuring(attribute.expression)) {
308+
if (attribute.expression) {
309+
for (const id of extract_identifiers_from_destructuring(attribute.expression)) {
310+
const binding = scope.declare(id, 'derived', 'const');
311+
bindings.push(binding);
312+
}
313+
} else {
314+
/** @type {import('estree').Identifier} */
315+
const id = {
316+
name: attribute.name,
317+
type: 'Identifier',
318+
start: attribute.start,
319+
end: attribute.end
320+
};
310321
const binding = scope.declare(id, 'derived', 'const');
311322
bindings.push(binding);
312323
}
313-
} else {
314-
/** @type {import('estree').Identifier} */
315-
const id = {
316-
name: attribute.name,
317-
type: 'Identifier',
318-
start: attribute.start,
319-
end: attribute.end
320-
};
321-
const binding = scope.declare(id, 'derived', 'const');
322-
bindings.push(binding);
324+
} else if (attribute.type === 'Attribute' && attribute.name === 'slot') {
325+
is_default_slot = false;
323326
}
324327
}
325-
return scope;
328+
329+
return /** @type {const} */ ([scope, is_default_slot]);
326330
}
327331

328332
walk(ast, state, {
@@ -364,13 +368,17 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
364368
Component(node, { state, visit, path }) {
365369
state.scope.reference(b.id(node.name), path);
366370

367-
// let:x from the default slot is a weird one:
368-
// Its scope only applies to children that are not slots themselves.
369371
for (const attribute of node.attributes) {
370372
visit(attribute);
371373
}
372374

373-
const scope = analyze_let_directives(node, state.scope);
375+
// let:x is super weird:
376+
// - for the default slot, its scope only applies to children that are not slots themselves
377+
// - for named slots, its scope applies to the component itself, too
378+
const [scope, is_default_slot] = analyze_let_directives(node, state.scope);
379+
if (!is_default_slot) {
380+
scopes.set(node, scope);
381+
}
374382

375383
for (const child of node.fragment.nodes) {
376384
if (

‎packages/svelte/tests/compiler-errors/samples/component-slot-duplicate-error-5/_config.js‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
import { test } from '../../test';
2+
3+
export default test({
4+
error: {
5+
code: 'duplicate-slot-name',
6+
message: "Duplicate slot name 'foo' in <Nested>"
7+
}
8+
});

‎packages/svelte/tests/compiler-errors/samples/component-slot-duplicate-error-5/main.svelte‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<script>
2+
import Nested from './irrelevant';
3+
import Inner from './irrelevant';
4+
</script>
5+
6+
<Nested>
7+
<Inner slot="foo">{value}</Inner>
8+
<Inner slot="foo">{value}</Inner>
9+
</Nested>

‎packages/svelte/tests/compiler-errors/samples/component-slot-duplicate-error-6/_config.js‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
import { test } from '../../test';
2+
3+
export default test({
4+
error: {
5+
code: 'duplicate-slot-name',
6+
message: "Duplicate slot name 'foo' in <Nested>"
7+
}
8+
});

‎packages/svelte/tests/compiler-errors/samples/component-slot-duplicate-error-6/main.svelte‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<script>
2+
import Nested from './irrelevant';
3+
import Inner from './irrelevant';
4+
</script>
5+
6+
<Nested>
7+
<p slot="foo">{value}</p>
8+
<Inner slot="foo">{value}</Inner>
9+
</Nested>

‎packages/svelte/tests/runtime-legacy/samples/component-slot-let-named-2/Nested.svelte‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<script>
2+
export let things;
3+
</script>
4+
5+
<div>
6+
{#each things as thing}
7+
<slot name="foo" {thing}/>
8+
{/each}
9+
</div>

0 commit comments

Comments
 (0)