fix: allow async `{@const}` in more places

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
pull/16643/head
Simon Holthausen 3 weeks ago
parent 1dcced59e7
commit d41d82189f

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: allow async `{@const}` in more places

@ -196,6 +196,51 @@ Cyclical dependency detected: %cycle%
`{@const}` must be the immediate child of `{#snippet}`, `{#if}`, `{:else if}`, `{:else}`, `{#each}`, `{:then}`, `{:catch}`, `<svelte:fragment>`, `<svelte:boundary` or `<Component>`
```
### const_tag_invalid_reference
```
The `{@const %name% = ...}` declaration is not available in this snippet
```
The following is an error:
```svelte
<svelte:boundary>
{@const foo = 'bar'}
{#snippet failed()}
{foo}
{/snippet}
</svelte:boundary>
```
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:
```svelte
<svelte:boundary>
{#snippet children()}
{@const foo = 'bar'}
{/snippet}
{#snippet failed()}
{foo}
{/snippet}
</svelte:boundary>
```
The same applies to components:
```svelte
<Component>
{@const foo = 'bar'}
{#snippet someProp()}
<!-- error -->
{foo}
{/snippet}
</Component>
```
### constant_assignment
```

@ -124,6 +124,49 @@
> `{@const}` must be the immediate child of `{#snippet}`, `{#if}`, `{:else if}`, `{:else}`, `{#each}`, `{:then}`, `{:catch}`, `<svelte:fragment>`, `<svelte:boundary` or `<Component>`
## const_tag_invalid_reference
> The `{@const %name% = ...}` declaration is not available in this snippet
The following is an error:
```svelte
<svelte:boundary>
{@const foo = 'bar'}
{#snippet failed()}
{foo}
{/snippet}
</svelte:boundary>
```
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:
```svelte
<svelte:boundary>
{#snippet children()}
{@const foo = 'bar'}
{/snippet}
{#snippet failed()}
{foo}
{/snippet}
</svelte:boundary>
```
The same applies to components:
```svelte
<Component>
{@const foo = 'bar'}
{#snippet someProp()}
<!-- error -->
{foo}
{/snippet}
</Component>
```
## debug_tag_invalid_arguments
> {@debug ...} arguments must be identifiers, not arbitrary expressions

@ -985,6 +985,16 @@ export function const_tag_invalid_placement(node) {
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`);
}
/**
* The `{@const %name% = ...}` declaration is not available in this snippet
* @param {null | number | NodeLike} node
* @param {string} name
* @returns {never}
*/
export function const_tag_invalid_reference(node, name) {
e(node, 'const_tag_invalid_reference', `The \`{@const ${name} = ...}\` declaration is not available in this snippet \nhttps://svelte.dev/e/const_tag_invalid_reference`);
}
/**
* {@debug ...} arguments must be identifiers, not arbitrary expressions
* @param {null | number | NodeLike} node

@ -7,6 +7,7 @@ import * as w from '../../../warnings.js';
import { is_rune } from '../../../../utils.js';
import { mark_subtree_dynamic } from './shared/fragment.js';
import { get_rune } from '../../scope.js';
import { is_component_node } from '../../nodes.js';
/**
* @param {Identifier} node
@ -155,5 +156,37 @@ export function Identifier(node, context) {
) {
w.reactive_declaration_module_script_dependency(node);
}
if (binding.metadata?.is_template_declaration && context.state.options.experimental.async) {
let snippet_name;
// Find out if this references a {@const ...} declaration of an implicit children snippet
// when it is itself inside a snippet block at the same level. If so, error.
for (let i = context.path.length - 1; i >= 0; i--) {
const parent = context.path[i];
const grand_parent = context.path[i - 1];
if (parent.type === 'SnippetBlock') {
snippet_name = parent.expression.name;
} else if (
snippet_name &&
grand_parent &&
parent.type === 'Fragment' &&
(is_component_node(grand_parent) ||
(grand_parent.type === 'SvelteBoundary' &&
(snippet_name === 'failed' || snippet_name === 'pending')))
) {
if (
is_component_node(grand_parent)
? grand_parent.metadata.scopes.default === binding.scope
: context.state.scopes.get(parent) === binding.scope
) {
e.const_tag_invalid_reference(node, node.name);
} else {
break;
}
}
}
}
}
}

@ -51,7 +51,6 @@ export function Fragment(node, context) {
const has_await = context.state.init !== null && (node.metadata.has_await || false);
const template_name = context.state.scope.root.unique('root'); // TODO infer name from parent
const unsuspend = b.id('$$unsuspend');
/** @type {Statement[]} */
const body = [];
@ -151,10 +150,6 @@ export function Fragment(node, context) {
}
}
if (has_await) {
body.push(b.var(unsuspend, b.call('$.suspend')));
}
body.push(...state.consts);
if (has_await) {
@ -182,8 +177,8 @@ export function Fragment(node, context) {
}
if (has_await) {
body.push(b.stmt(b.call(unsuspend)));
return b.block([b.stmt(b.call('$.async_body', b.arrow([], b.block(body), true)))]);
} else {
return b.block(body);
}
return b.block(body);
}

@ -39,40 +39,60 @@ export function SvelteBoundary(node, context) {
/** @type {Statement[]} */
const hoisted = [];
let has_const = false;
// const tags need to live inside the boundary, but might also be referenced in hoisted snippets.
// to resolve this we cheat: we duplicate const tags inside snippets
// We'll revert this behavior in the future, it was a mistake to allow this (Component snippets also don't do this).
for (const child of node.fragment.nodes) {
if (child.type === 'ConstTag') {
context.visit(child, { ...context.state, consts: const_tags });
has_const = true;
if (!context.state.options.experimental.async) {
context.visit(child, { ...context.state, consts: const_tags });
}
}
}
for (const child of node.fragment.nodes) {
if (child.type === 'ConstTag') {
if (context.state.options.experimental.async) {
nodes.push(child);
}
continue;
}
if (child.type === 'SnippetBlock') {
/** @type {Statement[]} */
const statements = [];
context.visit(child, { ...context.state, init: statements });
const snippet = /** @type {VariableDeclaration} */ (statements[0]);
const snippet_fn = dev
? // @ts-expect-error we know this shape is correct
snippet.declarations[0].init.arguments[1]
: snippet.declarations[0].init;
snippet_fn.body.body.unshift(
...const_tags.filter((node) => node.type === 'VariableDeclaration')
);
hoisted.push(snippet);
if (['failed', 'pending'].includes(child.expression.name)) {
props.properties.push(b.prop('init', child.expression, child.expression));
if (
context.state.options.experimental.async &&
has_const &&
!['failed', 'pending'].includes(child.expression.name)
) {
// we can't hoist snippets as they may reference const tags, so we just keep them in the fragment
nodes.push(child);
} else {
/** @type {Statement[]} */
const statements = [];
context.visit(child, { ...context.state, init: statements });
const snippet = /** @type {VariableDeclaration} */ (statements[0]);
const snippet_fn = dev
? // @ts-expect-error we know this shape is correct
snippet.declarations[0].init.arguments[1]
: snippet.declarations[0].init;
if (!context.state.options.experimental.async) {
snippet_fn.body.body.unshift(
...const_tags.filter((node) => node.type === 'VariableDeclaration')
);
}
if (['failed', 'pending'].includes(child.expression.name)) {
props.properties.push(b.prop('init', child.expression, child.expression));
}
hoisted.push(snippet);
}
continue;
@ -83,7 +103,9 @@ export function SvelteBoundary(node, context) {
const block = /** @type {BlockStatement} */ (context.visit({ ...node.fragment, nodes }));
block.body.unshift(...const_tags);
if (!context.state.options.experimental.async) {
block.body.unshift(...const_tags);
}
const boundary = b.stmt(
b.call('$.boundary', context.state.node, props, b.arrow([b.id('$$anchor')], block))

@ -23,6 +23,15 @@ export function is_element_node(node) {
return element_nodes.includes(node.type);
}
/**
* Returns true for all component-like nodes
* @param {AST.SvelteNode} node
* @returns {node is AST.Component | AST.SvelteComponent | AST.SvelteSelf}
*/
export function is_component_node(node) {
return ['Component', 'SvelteComponent', 'SvelteSelf'].includes(node.type);
}
/**
* @param {AST.RegularElement | AST.SvelteElement} node
* @returns {boolean}

@ -122,7 +122,7 @@ export class Binding {
/**
* Additional metadata, varies per binding type
* @type {null | { inside_rest?: boolean }}
* @type {null | { inside_rest?: boolean; is_template_declaration?: boolean }}
*/
metadata = null;
@ -1121,6 +1121,7 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
node.kind,
declarator.init
);
binding.metadata = { is_template_declaration: true };
bindings.push(binding);
}
}

@ -0,0 +1,10 @@
import { test } from '../../test';
export default test({
async: true,
error: {
code: 'const_tag_invalid_reference',
message: 'The `{@const foo = ...}` declaration is not available in this snippet ',
position: [376, 379]
}
});

@ -0,0 +1,32 @@
<svelte:options runes />
<!-- ok -->
<svelte:boundary>
{@const foo = 'bar'}
{#snippet other()}
{foo}
{/snippet}
{foo}
<svelte:boundary>
{#snippet failed()}
{foo}
{/snippet}
</svelte:boundary>
{#snippet failed()}
{@const foo = 'bar'}
{foo}
{/snippet}
</svelte:boundary>
<!-- error -->
<svelte:boundary>
{@const foo = 'bar'}
{#snippet failed()}
{foo}
{/snippet}
</svelte:boundary>

@ -0,0 +1,10 @@
import { test } from '../../test';
export default test({
async: true,
error: {
code: 'const_tag_invalid_reference',
message: 'The `{@const foo = ...}` declaration is not available in this snippet ',
position: [298, 301]
}
});

@ -0,0 +1,27 @@
<svelte:options runes />
<!-- ok -->
<Component>
{@const foo = 'bar'}
{foo}
<Component>
{#snippet prop()}
{foo}
{/snippet}
</Component>
{#snippet prop()}
{@const foo = 'bar'}
{foo}
{/snippet}
</Component>
<!-- error -->
<Component>
{@const foo = 'bar'}
{#snippet prop()}
{foo}
{/snippet}
</Component>

@ -5,6 +5,7 @@ import { suite, type BaseTest } from '../suite';
import { read_file } from '../helpers.js';
interface CompilerErrorTest extends BaseTest {
async?: boolean;
error: {
code: string;
message: string;
@ -29,7 +30,8 @@ const { test, run } = suite<CompilerErrorTest>((config, cwd) => {
try {
compile(read_file(`${cwd}/main.svelte`), {
generate: 'client'
generate: 'client',
experimental: { async: config.async ?? false }
});
} catch (e) {
const error = e as CompileError;

@ -7,6 +7,6 @@ export default test({
async test({ assert, target }) {
await tick();
assert.htmlEqual(target.innerHTML, `<h1>Hello, world!</h1>`);
assert.htmlEqual(target.innerHTML, `<h1>Hello, world!</h1> 5 01234`);
}
});

@ -3,6 +3,8 @@
</script>
<svelte:boundary>
{@const number = await Promise.resolve(5)}
{#snippet pending()}
<h1>Loading...</h1>
{/snippet}
@ -10,6 +12,14 @@
{#snippet greet()}
{@const greeting = await `Hello, ${name}!`}
<h1>{greeting}</h1>
{number}
{#if number > 4}
{@const length = await number}
{#each { length }, index}
{@const i = await index}
{i}
{/each}
{/if}
{/snippet}
{@render greet()}

@ -0,0 +1,18 @@
import { flushSync } from 'svelte';
import { test } from '../../test';
export default test({
skip_async: true,
html: '<button></button><p>2</p>',
mode: ['client'],
test({ target, assert }) {
const btn = target.querySelector('button');
const p = target.querySelector('p');
flushSync(() => {
btn?.click();
});
assert.equal(p?.innerHTML, '4');
}
});

@ -0,0 +1,14 @@
<script>
import FlakyComponent from "./FlakyComponent.svelte";
let test=$state(1);
</script>
<button onclick={()=>test++}></button>
<svelte:boundary>
{@const double = test * 2}
{#snippet failed()}
<p>{double}</p>
{/snippet}
<FlakyComponent />
</svelte:boundary>

@ -2,7 +2,7 @@ import { flushSync } from 'svelte';
import { test } from '../../test';
export default test({
html: '<button></button><p>2</p>',
html: '<button>increment</button><p>2</p>',
mode: ['client'],
test({ target, assert }) {
const btn = target.querySelector('button');

@ -1,14 +1,10 @@
<script>
import FlakyComponent from "./FlakyComponent.svelte";
let test=$state(1);
let count = $state(1);
</script>
<button onclick={()=>test++}></button>
<button onclick={()=>count++}>increment</button>
<svelte:boundary>
{@const double = test * 2}
{#snippet failed()}
<p>{double}</p>
{/snippet}
<FlakyComponent />
</svelte:boundary>
{@const double = count * 2}
<p>{double}</p>
</svelte:boundary>

@ -4,8 +4,6 @@
<svelte:boundary>
{@const x = a}
{#snippet failed()}
{x}
{/snippet}
{x}
<FlakyComponent />
</svelte:boundary>
</svelte:boundary>

Loading…
Cancel
Save