fix: correctly calculate `@const` blockers (#18039)

Move the calculation of blockers into the analysis phase and then only
push the right thunks in the transform phase - similar to how we already
do it with top level `$.run`

Fixes #18024

The solution is a tiny bit brittle (not much more than the top level one
we already have) and I tried to make it a bit more robust but ended up
in a rabbit hole in #18032 - we can revisit that solution once all the
old stuff is gone. Until then this is the most pragmatic/non-invasive
change.

---------

Co-authored-by: Rich Harris <rich.harris@vercel.com>
pull/17666/merge
Simon H 5 months ago committed by GitHub
parent 7be1a0247f
commit 3937ec03bd
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -0,0 +1,5 @@
---
'svelte': patch
---
fix: correctly calculate `@const` blockers

@ -2,6 +2,7 @@ import type { Scope } from '../scope.js';
import type { ComponentAnalysis, ReactiveStatement } from '../types.js'; import type { ComponentAnalysis, ReactiveStatement } from '../types.js';
import type { AST, StateField, ValidatedCompileOptions } from '#compiler'; import type { AST, StateField, ValidatedCompileOptions } from '#compiler';
import type { ExpressionMetadata } from '../nodes.js'; import type { ExpressionMetadata } from '../nodes.js';
import type { Identifier } from 'estree';
export interface AnalysisState { export interface AnalysisState {
scope: Scope; scope: Scope;
@ -33,6 +34,13 @@ export interface AnalysisState {
* Set when we're inside a `$derived(...)` expression (but not `$derived.by(...)`) or `@const` * Set when we're inside a `$derived(...)` expression (but not `$derived.by(...)`) or `@const`
*/ */
derived_function_depth: number; derived_function_depth: number;
/** Collected info about async `{@const }` declarations */
async_consts?: {
id: Identifier;
/** How many `@const` declarations there are (already) in this scope */
declaration_count: number;
};
} }
export type Context<State extends AnalysisState = AnalysisState> = import('zimmerframe').Context< export type Context<State extends AnalysisState = AnalysisState> = import('zimmerframe').Context<

@ -1,6 +1,7 @@
/** @import { AST } from '#compiler' */ /** @import { AST } from '#compiler' */
/** @import { Context } from '../types' */ /** @import { Context } from '../types' */
import * as e from '../../../errors.js'; import * as e from '../../../errors.js';
import * as b from '#compiler/builders';
import { validate_opening_tag } from './shared/utils.js'; import { validate_opening_tag } from './shared/utils.js';
/** /**
@ -42,4 +43,28 @@ export function ConstTag(node, context) {
function_depth: context.state.function_depth + 1, function_depth: context.state.function_depth + 1,
derived_function_depth: context.state.function_depth + 1 derived_function_depth: context.state.function_depth + 1
}); });
const has_await = node.metadata.expression.has_await;
const blockers = [...node.metadata.expression.dependencies]
.map((dep) => dep.blocker)
.filter((b) => b !== null && b.object !== context.state.async_consts?.id);
if (has_await || context.state.async_consts || blockers.length > 0) {
const run = (context.state.async_consts ??= {
id: context.state.analysis.root.unique('promises'),
declaration_count: 0
});
node.metadata.promises_id = run.id;
const bindings = context.state.scope.get_bindings(declaration);
// keep the counter in sync with the number of thunks pushed in ConstTag in transform
// TODO 6.0 once non-async and non-runes mode is gone investigate making this more robust
// via something like the approach in https://github.com/sveltejs/svelte/pull/18032
const length = run.declaration_count++;
const blocker = b.member(run.id, b.literal(length), true);
for (const binding of bindings) {
binding.blocker = blocker;
}
}
} }

@ -6,5 +6,5 @@
* @param {Context} context * @param {Context} context
*/ */
export function Fragment(node, context) { export function Fragment(node, context) {
context.next({ ...context.state, fragment: node }); context.next({ ...context.state, fragment: node, async_consts: undefined });
} }

@ -1,7 +1,6 @@
/** @import { Expression, Identifier, Pattern } from 'estree' */ /** @import { Expression, Identifier, Pattern, Statement } from 'estree' */
/** @import { AST } from '#compiler' */ /** @import { AST } from '#compiler' */
/** @import { ComponentContext } from '../types' */ /** @import { ComponentContext } from '../types' */
/** @import { ExpressionMetadata } from '../../../nodes.js' */
import { dev } from '../../../../state.js'; import { dev } from '../../../../state.js';
import { extract_identifiers } from '../../../../utils/ast.js'; import { extract_identifiers } from '../../../../utils/ast.js';
import * as b from '#compiler/builders'; import * as b from '#compiler/builders';
@ -27,13 +26,7 @@ export function ConstTag(node, context) {
context.state.transform[declaration.id.name] = { read: get_value }; context.state.transform[declaration.id.name] = { read: get_value };
add_const_declaration( add_const_declaration(context.state, declaration.id, expression, node.metadata);
context.state,
declaration.id,
expression,
node.metadata.expression,
context.state.scope.get_bindings(declaration)
);
} else { } else {
const identifiers = extract_identifiers(declaration.id); const identifiers = extract_identifiers(declaration.id);
const tmp = b.id(context.state.scope.generate('computed_const')); const tmp = b.id(context.state.scope.generate('computed_const'));
@ -70,13 +63,7 @@ export function ConstTag(node, context) {
expression = b.call('$.tag', expression, b.literal('[@const]')); expression = b.call('$.tag', expression, b.literal('[@const]'));
} }
add_const_declaration( add_const_declaration(context.state, tmp, expression, node.metadata);
context.state,
tmp,
expression,
node.metadata.expression,
context.state.scope.get_bindings(declaration)
);
for (const node of identifiers) { for (const node of identifiers) {
context.state.transform[node.name] = { context.state.transform[node.name] = {
@ -90,42 +77,40 @@ export function ConstTag(node, context) {
* @param {ComponentContext['state']} state * @param {ComponentContext['state']} state
* @param {Identifier} id * @param {Identifier} id
* @param {Expression} expression * @param {Expression} expression
* @param {ExpressionMetadata} metadata * @param {AST.ConstTag['metadata']} metadata
* @param {import('#compiler').Binding[]} bindings
*/ */
function add_const_declaration(state, id, expression, metadata, bindings) { function add_const_declaration(state, id, expression, metadata) {
// we need to eagerly evaluate the expression in order to hit any // we need to eagerly evaluate the expression in order to hit any
// 'Cannot access x before initialization' errors // 'Cannot access x before initialization' errors
const after = dev ? [b.stmt(b.call('$.get', id))] : []; const after = dev ? [b.stmt(b.call('$.get', id))] : [];
const has_await = metadata.has_await; const blockers = [...metadata.expression.dependencies]
const blockers = [...metadata.dependencies]
.map((dep) => dep.blocker) .map((dep) => dep.blocker)
.filter((b) => b !== null && b.object !== state.async_consts?.id); .filter((b) => b !== null && b.object !== state.async_consts?.id);
if (has_await || state.async_consts || blockers.length > 0) { if (metadata.promises_id) {
const run = (state.async_consts ??= { const run = (state.async_consts ??= {
id: b.id(state.scope.generate('promises')), id: metadata.promises_id,
thunks: [] thunks: []
}); });
state.consts.push(b.let(id)); state.consts.push(b.let(id));
const assignment = b.assignment('=', id, expression); /** @type {Statement | undefined} */
const body = after.length === 0 ? assignment : b.block([b.stmt(assignment), ...after]); let promise_stmt;
if (blockers.length === 1) { if (blockers.length === 1) {
run.thunks.push(b.thunk(b.member(/** @type {Expression} */ (blockers[0]), 'promise'))); promise_stmt = b.stmt(b.await(b.member(/** @type {Expression} */ (blockers[0]), 'promise')));
} else if (blockers.length > 0) { } else if (blockers.length > 0) {
run.thunks.push(b.thunk(b.call('$.wait', b.array(blockers)))); promise_stmt = b.stmt(b.await(b.call('$.wait', b.array(blockers))));
} }
run.thunks.push(b.thunk(body, has_await)); // keep the number of thunks pushed in sync with ConstTag in analysis phase
const assignment = b.assignment('=', id, expression);
const blocker = b.member(run.id, b.literal(run.thunks.length - 1), true); if (promise_stmt) {
run.thunks.push(b.thunk(b.block([promise_stmt, b.stmt(assignment)]), true));
for (const binding of bindings) { } else {
binding.blocker = blocker; run.thunks.push(b.thunk(assignment, metadata.expression.has_await));
} }
} else { } else {
state.consts.push(b.const(id, expression)); state.consts.push(b.const(id, expression));

@ -1,4 +1,4 @@
/** @import { Expression, Pattern } from 'estree' */ /** @import { Expression, Pattern, Statement } from 'estree' */
/** @import { AST } from '#compiler' */ /** @import { AST } from '#compiler' */
/** @import { ComponentContext } from '../types.js' */ /** @import { ComponentContext } from '../types.js' */
import * as b from '#compiler/builders'; import * as b from '#compiler/builders';
@ -12,36 +12,37 @@ export function ConstTag(node, context) {
const declaration = node.declaration.declarations[0]; const declaration = node.declaration.declarations[0];
const id = /** @type {Pattern} */ (context.visit(declaration.id)); const id = /** @type {Pattern} */ (context.visit(declaration.id));
const init = /** @type {Expression} */ (context.visit(declaration.init)); const init = /** @type {Expression} */ (context.visit(declaration.init));
const has_await = node.metadata.expression.has_await;
const blockers = [...node.metadata.expression.dependencies] const blockers = [...node.metadata.expression.dependencies]
.map((dep) => dep.blocker) .map((dep) => dep.blocker)
.filter((b) => b !== null && b.object !== context.state.async_consts?.id); .filter((b) => b !== null && b.object !== context.state.async_consts?.id);
if (has_await || context.state.async_consts || blockers.length > 0) { if (node.metadata.promises_id) {
const run = (context.state.async_consts ??= { const run = (context.state.async_consts ??= {
id: b.id(context.state.scope.generate('promises')), id: node.metadata.promises_id,
thunks: [] thunks: []
}); });
const identifiers = extract_identifiers(declaration.id); const identifiers = extract_identifiers(declaration.id);
const bindings = context.state.scope.get_bindings(declaration);
for (const identifier of identifiers) { for (const identifier of identifiers) {
context.state.init.push(b.let(identifier.name)); context.state.init.push(b.let(identifier.name));
} }
/** @type {Statement | undefined} */
let promise_stmt;
if (blockers.length === 1) { if (blockers.length === 1) {
run.thunks.push(b.thunk(/** @type {Expression} */ (blockers[0]))); promise_stmt = b.stmt(b.await(/** @type {Expression} */ (blockers[0])));
} else if (blockers.length > 0) { } else if (blockers.length > 0) {
run.thunks.push(b.thunk(b.call('Promise.all', b.array(blockers)))); promise_stmt = b.stmt(b.await(b.call('Promise.all', b.array(blockers))));
} }
// keep the number of thunks pushed in sync with ConstTag in analysis phase
const assignment = b.assignment('=', id, init); const assignment = b.assignment('=', id, init);
run.thunks.push(b.thunk(b.block([b.stmt(assignment)]), has_await)); if (promise_stmt) {
run.thunks.push(b.thunk(b.block([promise_stmt, b.stmt(assignment)]), true));
const blocker = b.member(run.id, b.literal(run.thunks.length - 1), true); } else {
for (const binding of bindings) { run.thunks.push(b.thunk(assignment, node.metadata.expression.has_await));
binding.blocker = blocker;
} }
} else { } else {
context.state.init.push(b.const(id, init)); context.state.init.push(b.const(id, init));

@ -155,6 +155,8 @@ export namespace AST {
/** @internal */ /** @internal */
metadata: { metadata: {
expression: ExpressionMetadata; expression: ExpressionMetadata;
/** If this const tag contains an await expression, or needs to wait on other async, this is set */
promises_id?: Identifier;
}; };
} }

@ -0,0 +1,9 @@
import { tick } from 'svelte';
import { test } from '../../test';
export default test({
async test({ assert, target }) {
await tick();
assert.htmlEqual(target.innerHTML, '<p>data</p>');
}
});

@ -0,0 +1,16 @@
<script>
let d = $derived(await Promise.resolve({ data: "data", hasData: true }));
let showFetchCta = $derived(d.hasData)
</script>
{#if d}
{@const {data, hasData} = d}
{#if hasData}
<p>{data}</p>
{:else if showFetchCta}
<p>Fetch now</p>
{:else}
<p>No data</p>
{/if}
{/if}

@ -7,16 +7,7 @@ export default function Async_const($$renderer) {
let a; let a;
let b; let b;
var promises = $$renderer.run([async () => a = (await $.save(1))(), () => b = a + 1]);
var promises = $$renderer.run([
async () => {
a = (await $.save(1))();
},
() => {
b = a + 1;
}
]);
$$renderer.push(`<p>`); $$renderer.push(`<p>`);
$$renderer.async([promises[1]], ($$renderer) => $$renderer.push(() => $.escape(b))); $$renderer.async([promises[1]], ($$renderer) => $$renderer.push(() => $.escape(b)));

@ -28,25 +28,15 @@ export default function Async_in_derived($$renderer, $$props) {
let no2; let no2;
var promises = $$renderer.run([ var promises = $$renderer.run([
async () => { async () => yes1 = (await $.save(1))(),
yes1 = (await $.save(1))(); async () => yes2 = foo((await $.save(1))()),
}, () => no1 = (async () => {
async () => {
yes2 = foo((await $.save(1))());
},
() => {
no1 = (async () => {
return await 1; return await 1;
})(); })(),
},
() => { () => no2 = (async () => {
no2 = (async () => {
return await 1; return await 1;
})(); })()
}
]); ]);
} else { } else {
$$renderer.push('<!--[-1-->'); $$renderer.push('<!--[-1-->');

Loading…
Cancel
Save