Skip to content

Commit a60995a

Browse files
authored
fix: allow async {@const} in more places (#16643)
Implemented by reusing the `async_body` function inside `Fragment.js`. Also removes the ability to reference a `{@const ...}` of an implicit child inside a boundary pending/failed snippet: - existing duplication of consts can have unintended side effects, e.g. async consts would unexpectedly called multiple times - what if a const is the reason for the failure of a boundary, but is then referenced in the failed snippet? - what if an async const is referenced in a pending snippet? deadlock - inconsistent with how it behaves for components where this already does not work Implemented via the experimental flag so the behavior change only applies there as this is a breaking change strictly speaking. Also added a compiler error for this. closes #16462
1 parent 1dcced5 commit a60995a

File tree

22 files changed

+328
-48
lines changed

22 files changed

+328
-48
lines changed

.changeset/light-camels-push.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: allow async `{@const}` in more places

documentation/docs/98-reference/.generated/compile-errors.md

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,51 @@ Cyclical dependency detected: %cycle%
196196
`{@const}` must be the immediate child of `{#snippet}`, `{#if}`, `{:else if}`, `{:else}`, `{#each}`, `{:then}`, `{:catch}`, `<svelte:fragment>`, `<svelte:boundary` or `<Component>`
197197
```
198198

199+
### const_tag_invalid_reference
200+
201+
```
202+
The `{@const %name% = ...}` declaration is not available in this snippet
203+
```
204+
205+
The following is an error:
206+
207+
```svelte
208+
<svelte:boundary>
209+
{@const foo = 'bar'}
210+
211+
{#snippet failed()}
212+
{foo}
213+
{/snippet}
214+
</svelte:boundary>
215+
```
216+
217+
Here, `foo` is not available inside `failed`. The top level code inside `<svelte:boundary>` becomes part of the implicit `children` snippet, in other words the above code is equivalent to this:
218+
219+
```svelte
220+
<svelte:boundary>
221+
{#snippet children()}
222+
{@const foo = 'bar'}
223+
{/snippet}
224+
225+
{#snippet failed()}
226+
{foo}
227+
{/snippet}
228+
</svelte:boundary>
229+
```
230+
231+
The same applies to components:
232+
233+
```svelte
234+
<Component>
235+
{@const foo = 'bar'}
236+
237+
{#snippet someProp()}
238+
<!-- error -->
239+
{foo}
240+
{/snippet}
241+
</Component>
242+
```
243+
199244
### constant_assignment
200245

201246
```

packages/svelte/messages/compile-errors/template.md

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,49 @@
124124

125125
> `{@const}` must be the immediate child of `{#snippet}`, `{#if}`, `{:else if}`, `{:else}`, `{#each}`, `{:then}`, `{:catch}`, `<svelte:fragment>`, `<svelte:boundary` or `<Component>`
126126
127+
## const_tag_invalid_reference
128+
129+
> The `{@const %name% = ...}` declaration is not available in this snippet
130+
131+
The following is an error:
132+
133+
```svelte
134+
<svelte:boundary>
135+
{@const foo = 'bar'}
136+
137+
{#snippet failed()}
138+
{foo}
139+
{/snippet}
140+
</svelte:boundary>
141+
```
142+
143+
Here, `foo` is not available inside `failed`. The top level code inside `<svelte:boundary>` becomes part of the implicit `children` snippet, in other words the above code is equivalent to this:
144+
145+
```svelte
146+
<svelte:boundary>
147+
{#snippet children()}
148+
{@const foo = 'bar'}
149+
{/snippet}
150+
151+
{#snippet failed()}
152+
{foo}
153+
{/snippet}
154+
</svelte:boundary>
155+
```
156+
157+
The same applies to components:
158+
159+
```svelte
160+
<Component>
161+
{@const foo = 'bar'}
162+
163+
{#snippet someProp()}
164+
<!-- error -->
165+
{foo}
166+
{/snippet}
167+
</Component>
168+
```
169+
127170
## debug_tag_invalid_arguments
128171

129172
> {@debug ...} arguments must be identifiers, not arbitrary expressions

packages/svelte/src/compiler/errors.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -985,6 +985,16 @@ export function const_tag_invalid_placement(node) {
985985
e(node, 'const_tag_invalid_placement', `\`{@const}\` must be the immediate child of \`{#snippet}\`, \`{#if}\`, \`{:else if}\`, \`{:else}\`, \`{#each}\`, \`{:then}\`, \`{:catch}\`, \`<svelte:fragment>\`, \`<svelte:boundary\` or \`<Component>\`\nhttps://svelte.dev/e/const_tag_invalid_placement`);
986986
}
987987

988+
/**
989+
* The `{@const %name% = ...}` declaration is not available in this snippet
990+
* @param {null | number | NodeLike} node
991+
* @param {string} name
992+
* @returns {never}
993+
*/
994+
export function const_tag_invalid_reference(node, name) {
995+
e(node, 'const_tag_invalid_reference', `The \`{@const ${name} = ...}\` declaration is not available in this snippet \nhttps://svelte.dev/e/const_tag_invalid_reference`);
996+
}
997+
988998
/**
989999
* {@debug ...} arguments must be identifiers, not arbitrary expressions
9901000
* @param {null | number | NodeLike} node

packages/svelte/src/compiler/phases/2-analyze/visitors/Identifier.js

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import * as w from '../../../warnings.js';
77
import { is_rune } from '../../../../utils.js';
88
import { mark_subtree_dynamic } from './shared/fragment.js';
99
import { get_rune } from '../../scope.js';
10+
import { is_component_node } from '../../nodes.js';
1011

1112
/**
1213
* @param {Identifier} node
@@ -155,5 +156,37 @@ export function Identifier(node, context) {
155156
) {
156157
w.reactive_declaration_module_script_dependency(node);
157158
}
159+
160+
if (binding.metadata?.is_template_declaration && context.state.options.experimental.async) {
161+
let snippet_name;
162+
163+
// Find out if this references a {@const ...} declaration of an implicit children snippet
164+
// when it is itself inside a snippet block at the same level. If so, error.
165+
for (let i = context.path.length - 1; i >= 0; i--) {
166+
const parent = context.path[i];
167+
const grand_parent = context.path[i - 1];
168+
169+
if (parent.type === 'SnippetBlock') {
170+
snippet_name = parent.expression.name;
171+
} else if (
172+
snippet_name &&
173+
grand_parent &&
174+
parent.type === 'Fragment' &&
175+
(is_component_node(grand_parent) ||
176+
(grand_parent.type === 'SvelteBoundary' &&
177+
(snippet_name === 'failed' || snippet_name === 'pending')))
178+
) {
179+
if (
180+
is_component_node(grand_parent)
181+
? grand_parent.metadata.scopes.default === binding.scope
182+
: context.state.scopes.get(parent) === binding.scope
183+
) {
184+
e.const_tag_invalid_reference(node, node.name);
185+
} else {
186+
break;
187+
}
188+
}
189+
}
190+
}
158191
}
159192
}

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

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@ export function Fragment(node, context) {
5151
const has_await = context.state.init !== null && (node.metadata.has_await || false);
5252

5353
const template_name = context.state.scope.root.unique('root'); // TODO infer name from parent
54-
const unsuspend = b.id('$$unsuspend');
5554

5655
/** @type {Statement[]} */
5756
const body = [];
@@ -151,10 +150,6 @@ export function Fragment(node, context) {
151150
}
152151
}
153152

154-
if (has_await) {
155-
body.push(b.var(unsuspend, b.call('$.suspend')));
156-
}
157-
158153
body.push(...state.consts);
159154

160155
if (has_await) {
@@ -182,8 +177,8 @@ export function Fragment(node, context) {
182177
}
183178

184179
if (has_await) {
185-
body.push(b.stmt(b.call(unsuspend)));
180+
return b.block([b.stmt(b.call('$.async_body', b.arrow([], b.block(body), true)))]);
181+
} else {
182+
return b.block(body);
186183
}
187-
188-
return b.block(body);
189184
}

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

Lines changed: 44 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -39,40 +39,60 @@ export function SvelteBoundary(node, context) {
3939
/** @type {Statement[]} */
4040
const hoisted = [];
4141

42+
let has_const = false;
43+
4244
// const tags need to live inside the boundary, but might also be referenced in hoisted snippets.
4345
// to resolve this we cheat: we duplicate const tags inside snippets
46+
// We'll revert this behavior in the future, it was a mistake to allow this (Component snippets also don't do this).
4447
for (const child of node.fragment.nodes) {
4548
if (child.type === 'ConstTag') {
46-
context.visit(child, { ...context.state, consts: const_tags });
49+
has_const = true;
50+
if (!context.state.options.experimental.async) {
51+
context.visit(child, { ...context.state, consts: const_tags });
52+
}
4753
}
4854
}
4955

5056
for (const child of node.fragment.nodes) {
5157
if (child.type === 'ConstTag') {
58+
if (context.state.options.experimental.async) {
59+
nodes.push(child);
60+
}
5261
continue;
5362
}
5463

5564
if (child.type === 'SnippetBlock') {
56-
/** @type {Statement[]} */
57-
const statements = [];
58-
59-
context.visit(child, { ...context.state, init: statements });
60-
61-
const snippet = /** @type {VariableDeclaration} */ (statements[0]);
62-
63-
const snippet_fn = dev
64-
? // @ts-expect-error we know this shape is correct
65-
snippet.declarations[0].init.arguments[1]
66-
: snippet.declarations[0].init;
67-
68-
snippet_fn.body.body.unshift(
69-
...const_tags.filter((node) => node.type === 'VariableDeclaration')
70-
);
71-
72-
hoisted.push(snippet);
73-
74-
if (['failed', 'pending'].includes(child.expression.name)) {
75-
props.properties.push(b.prop('init', child.expression, child.expression));
65+
if (
66+
context.state.options.experimental.async &&
67+
has_const &&
68+
!['failed', 'pending'].includes(child.expression.name)
69+
) {
70+
// we can't hoist snippets as they may reference const tags, so we just keep them in the fragment
71+
nodes.push(child);
72+
} else {
73+
/** @type {Statement[]} */
74+
const statements = [];
75+
76+
context.visit(child, { ...context.state, init: statements });
77+
78+
const snippet = /** @type {VariableDeclaration} */ (statements[0]);
79+
80+
const snippet_fn = dev
81+
? // @ts-expect-error we know this shape is correct
82+
snippet.declarations[0].init.arguments[1]
83+
: snippet.declarations[0].init;
84+
85+
if (!context.state.options.experimental.async) {
86+
snippet_fn.body.body.unshift(
87+
...const_tags.filter((node) => node.type === 'VariableDeclaration')
88+
);
89+
}
90+
91+
if (['failed', 'pending'].includes(child.expression.name)) {
92+
props.properties.push(b.prop('init', child.expression, child.expression));
93+
}
94+
95+
hoisted.push(snippet);
7696
}
7797

7898
continue;
@@ -83,7 +103,9 @@ export function SvelteBoundary(node, context) {
83103

84104
const block = /** @type {BlockStatement} */ (context.visit({ ...node.fragment, nodes }));
85105

86-
block.body.unshift(...const_tags);
106+
if (!context.state.options.experimental.async) {
107+
block.body.unshift(...const_tags);
108+
}
87109

88110
const boundary = b.stmt(
89111
b.call('$.boundary', context.state.node, props, b.arrow([b.id('$$anchor')], block))

packages/svelte/src/compiler/phases/nodes.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,15 @@ export function is_element_node(node) {
2323
return element_nodes.includes(node.type);
2424
}
2525

26+
/**
27+
* Returns true for all component-like nodes
28+
* @param {AST.SvelteNode} node
29+
* @returns {node is AST.Component | AST.SvelteComponent | AST.SvelteSelf}
30+
*/
31+
export function is_component_node(node) {
32+
return ['Component', 'SvelteComponent', 'SvelteSelf'].includes(node.type);
33+
}
34+
2635
/**
2736
* @param {AST.RegularElement | AST.SvelteElement} node
2837
* @returns {boolean}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ export class Binding {
122122

123123
/**
124124
* Additional metadata, varies per binding type
125-
* @type {null | { inside_rest?: boolean }}
125+
* @type {null | { inside_rest?: boolean; is_template_declaration?: boolean }}
126126
*/
127127
metadata = null;
128128

@@ -1121,6 +1121,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
11211121
node.kind,
11221122
declarator.init
11231123
);
1124+
binding.metadata = { is_template_declaration: true };
11241125
bindings.push(binding);
11251126
}
11261127
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
import { test } from '../../test';
2+
3+
export default test({
4+
async: true,
5+
error: {
6+
code: 'const_tag_invalid_reference',
7+
message: 'The `{@const foo = ...}` declaration is not available in this snippet ',
8+
position: [376, 379]
9+
}
10+
});

0 commit comments

Comments
 (0)