diff --git a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js index dd8e54fbfc..d1b7792272 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js @@ -6,7 +6,7 @@ import { walk } from 'zimmerframe'; import { set_scope } from '../../scope.js'; import { extract_identifiers } from '../../../utils/ast.js'; import * as b from '#compiler/builders'; -import { dev, filename } from '../../../state.js'; +import { component_name, dev, filename } from '../../../state.js'; import { render_stylesheet } from '../css/index.js'; import { AssignmentExpression } from './visitors/AssignmentExpression.js'; import { AwaitBlock } from './visitors/AwaitBlock.js'; @@ -40,7 +40,7 @@ import { TitleElement } from './visitors/TitleElement.js'; import { UpdateExpression } from './visitors/UpdateExpression.js'; import { VariableDeclaration } from './visitors/VariableDeclaration.js'; import { SvelteBoundary } from './visitors/SvelteBoundary.js'; -import { call_child_payload } from './visitors/shared/utils.js'; +import { call_child_payload, call_component_payload } from './visitors/shared/utils.js'; /** @type {Visitors} */ const global_visitors = { @@ -260,8 +260,9 @@ export function server_component(analysis, options) { let should_inject_context = dev || analysis.needs_context; if (should_inject_context) { - component_block.body.unshift(b.stmt(b.call('$.push', dev && b.id(analysis.name)))); - component_block.body.push(b.stmt(b.call('$.pop'))); + component_block = b.block([ + call_component_payload(component_block, dev && b.id(component_name)) + ]); } if (analysis.uses_rest_props) { diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/utils.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/utils.js index 166845b62f..a48c19ae7c 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/utils.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/utils.js @@ -270,6 +270,17 @@ export function call_child_payload(body, async) { return b.stmt(b.call('$$payload.child', b.arrow([b.id('$$payload')], body, async))); } +/** + * @param {BlockStatement | Expression} body + * @param {Identifier | false} component_fn_id + * @returns {Statement} + */ +export function call_component_payload(body, component_fn_id) { + return b.stmt( + b.call('$$payload.component', b.arrow([b.id('$$payload')], body, false), component_fn_id) + ); +} + export class PromiseOptimiser { /** @type {Expression[]} */ expressions = []; diff --git a/packages/svelte/src/index-server.js b/packages/svelte/src/index-server.js index 019356a272..a1c072a1ca 100644 --- a/packages/svelte/src/index-server.js +++ b/packages/svelte/src/index-server.js @@ -5,8 +5,7 @@ import * as e from './internal/server/errors.js'; /** @param {() => void} fn */ export function onDestroy(fn) { - var context = /** @type {SSRContext} */ (ssr_context); - (context.d ??= []).push(fn); + /** @type {SSRContext} */ (ssr_context).r.on_destroy(fn); } export { diff --git a/packages/svelte/src/internal/server/context.js b/packages/svelte/src/internal/server/context.js index ca5d7b648f..03fc8672be 100644 --- a/packages/svelte/src/internal/server/context.js +++ b/packages/svelte/src/internal/server/context.js @@ -1,6 +1,5 @@ /** @import { SSRContext } from '#server' */ import { DEV } from 'esm-env'; -import { async_on_destroy, on_destroy } from './index.js'; import * as e from './errors.js'; /** @type {SSRContext | null} */ @@ -63,7 +62,7 @@ function get_or_init_context_map(name) { * @param {Function} [fn] */ export function push(fn) { - ssr_context = { p: ssr_context, c: null, d: null }; + ssr_context = { p: ssr_context, c: null, r: null }; if (DEV) { ssr_context.function = fn; @@ -72,17 +71,7 @@ export function push(fn) { } export function pop() { - var context = /** @type {SSRContext} */ (ssr_context); - - var ondestroy = context.d; - - if (ondestroy) { - on_destroy.push(...ondestroy); - // TODO this is probably actually broken - async_on_destroy.push(...ondestroy); - } - - ssr_context = context.p; + ssr_context = /** @type {SSRContext} */ (ssr_context).p; } /** diff --git a/packages/svelte/src/internal/server/index.js b/packages/svelte/src/internal/server/index.js index e8c6211683..b9c2339e81 100644 --- a/packages/svelte/src/internal/server/index.js +++ b/packages/svelte/src/internal/server/index.js @@ -53,12 +53,6 @@ export function element(payload, tag, attributes_fn = noop, children_fn = noop) payload.push(''); } -/** - * Array of `onDestroy` callbacks that should be called at the end of the server render function - * @type {Function[]} - */ -export let on_destroy = []; - /** * Only available on the server and when compiling with the `server` option. * Takes a component and returns an object with `body` and `head` properties on it, which you can use to populate the HTML when server-rendering your app. @@ -75,8 +69,6 @@ export function render(component, options = {}) { new TreeState('sync', options.idPrefix ? options.idPrefix + '-' : '') ); - const prev_on_destroy = on_destroy; - on_destroy = []; payload.push(BLOCK_OPEN); if (options.context) { @@ -92,10 +84,9 @@ export function render(component, options = {}) { } payload.push(BLOCK_CLOSE); - for (const cleanup of on_destroy) cleanup(); - on_destroy = prev_on_destroy; let { head, body } = payload.collect(); + for (const cleanup of payload.collect_on_destroy()) cleanup(); head += payload.global.head.title.value; body = BLOCK_OPEN + body + BLOCK_CLOSE; // this inserts a fake boundary so hydration matches @@ -115,13 +106,6 @@ export function render(component, options = {}) { } } -/** - * TODO THIS NEEDS TO ACTUALLY BE DONE - * Array of `onDestroy` callbacks that should be called at the end of the server render function - * @type {Function[]} - */ -export let async_on_destroy = []; - /** * Only available on the server and when compiling with the `server` option. * Takes a component and returns an object with `body` and `head` properties on it, which you can use to populate the HTML when server-rendering your app. @@ -138,8 +122,6 @@ export async function render_async(component, options = {}) { new TreeState('async', options.idPrefix ? options.idPrefix + '-' : '') ); - const prev_on_destroy = async_on_destroy; - async_on_destroy = []; payload.push(BLOCK_OPEN); if (options.context) { @@ -155,10 +137,9 @@ export async function render_async(component, options = {}) { } payload.push(BLOCK_CLOSE); - for (const cleanup of async_on_destroy) cleanup(); - async_on_destroy = prev_on_destroy; let { head, body } = await payload.collect_async(); + for (const cleanup of payload.collect_on_destroy()) cleanup(); head += payload.global.head.title.value; body = BLOCK_OPEN + body + BLOCK_CLOSE; // this inserts a fake boundary so hydration matches diff --git a/packages/svelte/src/internal/server/payload.js b/packages/svelte/src/internal/server/payload.js index 25d339b853..29b69c99b5 100644 --- a/packages/svelte/src/internal/server/payload.js +++ b/packages/svelte/src/internal/server/payload.js @@ -6,9 +6,11 @@ * @typedef {T | Promise} MaybePromise */ /** - * @typedef {string | Payload | Promise} PayloadItem + * @typedef {string | Payload} PayloadItem */ +import { pop, push, set_ssr_context, ssr_context } from './context'; + /** * Payloads are basically a tree of `string | Payload`s, where each `Payload` in the tree represents * work that may or may not have completed. A payload can be {@link collect}ed to aggregate the @@ -25,6 +27,18 @@ export class Payload { */ #out = []; + /** + * Any `onDestroy` callbacks registered during execution of this payload. + * @type {(() => void)[] | undefined} + */ + #on_destroy = undefined; + + /** + * Whether this payload is a component body. + * @type {boolean} + */ + #is_component_body = false; + /** * The type of string content that this payload is accumulating. * @type {PayloadType} @@ -82,17 +96,22 @@ export class Payload { */ child(render, type) { const child = new Payload(this.global, this.local, this, type); - this.#out.push(child); - const result = render(child); - if (result instanceof Promise) { - if (this.global.mode === 'sync') { - // TODO more-proper error - throw new Error('Encountered an asynchronous component while rendering synchronously'); - } - // just to avoid unhandled promise rejections -- we'll end up throwing in `then` if something fails - result.catch(() => {}); - child.promises.initial = result; - } + this.#run_child(child, render); + } + + /** + * Create a component payload. The component payload inherits the state from the parent, + * but has its own content. It is treated as an ordering boundary for ondestroy callbacks. + * @param {(tree: Payload) => MaybePromise} render + * @param {Function} [fn] + * @returns {void} + */ + component(render, fn) { + push(fn); + const child = new Payload(this.global, this.local, this); + child.#is_component_body = true; + this.#run_child(child, render); + pop(); } /** @@ -100,11 +119,7 @@ export class Payload { */ push(content) { if (typeof content === 'function') { - if (this.global.mode === 'sync') { - // TODO more-proper error - throw new Error('Encountered an asynchronous component while rendering synchronously'); - } - this.#out.push(content()); + this.child(async (payload) => payload.push(await content())); } else { this.#out.push(content); } @@ -126,6 +141,13 @@ export class Payload { } } + /** + * @param {() => void} fn + */ + on_destroy(fn) { + (this.#on_destroy ??= []).push(fn); + } + /** * @returns {number[]} */ @@ -150,6 +172,84 @@ export class Payload { return Payload.#collect_content_async([this], this.type); } + /** + * Collect all of the `onDestroy` callbacks regsitered during rendering. In an async context, this is only safe to call + * after awaiting `collect_async`. + * + * Child payloads are "porous" and don't affect execution order, but component body payloads + * create ordering boundaries. Within a payload, callbacks run in order until hitting a component boundary. + * @returns {Iterable<() => void>} + */ + *collect_on_destroy() { + const payload_children = this.#out.filter((child) => child instanceof Payload); + + // First, do a depth-first search to find and process all component bodies + // This includes components nested inside regular payloads + for (const child of payload_children) { + yield* this.#collect_component_callbacks(child); + } + + // Then, if this is a component body, yield its own callbacks + if (this.#is_component_body && this.#on_destroy) { + for (const fn of this.#on_destroy) { + yield fn; + } + } + + // Finally, collect callbacks from regular (porous) payloads + for (const child of payload_children) { + if (!child.#is_component_body) { + yield* this.#collect_regular_callbacks(child); + } + } + + // If this is NOT a component body, yield callbacks after all processing + if (!this.#is_component_body && this.#on_destroy) { + for (const fn of this.#on_destroy) { + yield fn; + } + } + } + + /** + * Helper method to collect only component body callbacks in depth-first order + * @param {Payload} payload + * @returns {Iterable<() => void>} + */ + *#collect_component_callbacks(payload) { + if (payload.#is_component_body) { + // This is a component body - process it + yield* payload.collect_on_destroy(); + } else { + // This is a regular payload - look for nested components + const children = payload.#out.filter((child) => child instanceof Payload); + for (const child of children) { + yield* this.#collect_component_callbacks(child); + } + } + } + + /** + * Helper method to collect callbacks from regular (porous) payloads + * @param {Payload} payload + * @returns {Iterable<() => void>} + */ + *#collect_regular_callbacks(payload) { + if (payload.#on_destroy) { + for (const fn of payload.#on_destroy) { + yield fn; + } + } + + // Recursively collect from regular payload children + const children = payload.#out + .filter((child) => child instanceof Payload) + .filter((child) => !child.#is_component_body); + for (const child of children) { + yield* this.#collect_regular_callbacks(child); + } + } + copy() { const copy = new Payload(this.global, this.local, this.#parent, this.type); copy.#out = this.#out.map((item) => (item instanceof Payload ? item.copy() : item)); @@ -198,6 +298,31 @@ export class Payload { } } + /** + * @param {Payload} child_payload + * @param {(tree: Payload) => MaybePromise} render + * @returns {void} + */ + #run_child(child_payload, render) { + this.#out.push(child_payload); + set_ssr_context({ + ...ssr_context, + p: ssr_context?.p ?? null, + c: ssr_context?.c ?? null, + r: child_payload + }); + const result = render(child_payload); + if (result instanceof Promise) { + if (this.global.mode === 'sync') { + // TODO more-proper error + throw new Error('Encountered an asynchronous component while rendering synchronously'); + } + // just to avoid unhandled promise rejections -- we'll end up throwing in `collect_async` if something fails + result.catch(() => {}); + child_payload.promises.initial = result; + } + } + /** * @param {(content: AccumulatedContent) => AccumulatedContent | Promise} fn * @param {Payload} child @@ -223,8 +348,6 @@ export class Payload { content[current_type] += item; } else if (item instanceof Payload) { Payload.#collect_content(item.#out, item.type, content); - } else { - throw new Error('invariant: should never reach this'); } } return content; @@ -240,7 +363,9 @@ export class Payload { static async #collect_content_async(items, current_type, content = { head: '', body: '' }) { // no danger to sequentially awaiting stuff in here; all of the work is already kicked off for (const item of items) { - if (item instanceof Payload) { + if (typeof item === 'string') { + content[current_type] += item; + } else { if (item.promises.initial) { // this represents the async function that's modifying this payload. // we can't do anything until it's done and we know our `out` array is complete. @@ -251,8 +376,6 @@ export class Payload { await followup; } await Payload.#collect_content_async(item.#out, item.type, content); - } else { - content[current_type] += await item; } } return content; diff --git a/packages/svelte/src/internal/server/payload.test.ts b/packages/svelte/src/internal/server/payload.test.ts index 8e2c87e040..0997a5675b 100644 --- a/packages/svelte/src/internal/server/payload.test.ts +++ b/packages/svelte/src/internal/server/payload.test.ts @@ -335,3 +335,34 @@ test('push async functions are not supported in sync context', () => { payload.collect(); }).toThrow(); }); + +test('collect_on_destroy yields callbacks in the correct order', async () => { + const payload = new Payload(new TreeState('async')); + const destroyed: string[] = []; + payload.component((payload) => { + payload.on_destroy(() => destroyed.push('a')); + // children should not alter relative order + payload.child(async ($$payload) => { + await Promise.resolve(); + $$payload.on_destroy(() => destroyed.push('b')); + $$payload.on_destroy(() => destroyed.push('b*')); + + // but child components should + $$payload.component(($$inner) => { + $$inner.on_destroy(() => destroyed.push('c')); + }); + }); + payload.child((payload) => { + payload.on_destroy(() => destroyed.push('d')); + }); + payload.component((payload) => { + payload.on_destroy(() => destroyed.push('e')); + }); + }); + + await payload.collect_async(); + for (const callback of payload.collect_on_destroy()) { + callback(); + } + assert.deepEqual(destroyed, ['c', 'e', 'a', 'b', 'b*', 'd']); +}); diff --git a/packages/svelte/src/internal/server/types.d.ts b/packages/svelte/src/internal/server/types.d.ts index cbf2c45fba..36f3de3a1e 100644 --- a/packages/svelte/src/internal/server/types.d.ts +++ b/packages/svelte/src/internal/server/types.d.ts @@ -1,12 +1,13 @@ import type { Element } from './dev'; +import type { Payload } from './payload'; export interface SSRContext { /** parent */ p: null | SSRContext; /** component context */ c: null | Map; - /** ondestroy */ - d: null | Array<() => void>; + /** payload (renderer) */ + r: null | Payload; /** dev mode only: the current component function */ function?: any; /** dev mode only: the current element */ diff --git a/packages/svelte/tests/runtime-legacy/samples/after-render-prevents-loop/_config.js b/packages/svelte/tests/runtime-legacy/samples/after-render-prevents-loop/_config.js index 39c0ef7f53..7a0943c3f4 100644 --- a/packages/svelte/tests/runtime-legacy/samples/after-render-prevents-loop/_config.js +++ b/packages/svelte/tests/runtime-legacy/samples/after-render-prevents-loop/_config.js @@ -1,7 +1,7 @@ import { test } from '../../test'; export default test({ - skip_mode: ['server'], + skip_mode: ['server', 'async-server'], get props() { return { value: 'hello!' }; diff --git a/packages/svelte/tests/runtime-legacy/samples/after-render-triggers-update/_config.js b/packages/svelte/tests/runtime-legacy/samples/after-render-triggers-update/_config.js index 39c0ef7f53..7a0943c3f4 100644 --- a/packages/svelte/tests/runtime-legacy/samples/after-render-triggers-update/_config.js +++ b/packages/svelte/tests/runtime-legacy/samples/after-render-triggers-update/_config.js @@ -1,7 +1,7 @@ import { test } from '../../test'; export default test({ - skip_mode: ['server'], + skip_mode: ['server', 'async-server'], get props() { return { value: 'hello!' }; diff --git a/packages/svelte/tests/runtime-legacy/samples/before-render-prevents-loop/_config.js b/packages/svelte/tests/runtime-legacy/samples/before-render-prevents-loop/_config.js index 39c0ef7f53..7a0943c3f4 100644 --- a/packages/svelte/tests/runtime-legacy/samples/before-render-prevents-loop/_config.js +++ b/packages/svelte/tests/runtime-legacy/samples/before-render-prevents-loop/_config.js @@ -1,7 +1,7 @@ import { test } from '../../test'; export default test({ - skip_mode: ['server'], + skip_mode: ['server', 'async-server'], get props() { return { value: 'hello!' }; diff --git a/packages/svelte/tests/runtime-legacy/samples/constructor-prefer-passed-context/_config.js b/packages/svelte/tests/runtime-legacy/samples/constructor-prefer-passed-context/_config.js index e05bb35637..7c49960afd 100644 --- a/packages/svelte/tests/runtime-legacy/samples/constructor-prefer-passed-context/_config.js +++ b/packages/svelte/tests/runtime-legacy/samples/constructor-prefer-passed-context/_config.js @@ -1,7 +1,7 @@ import { test } from '../../test'; export default test({ - skip_mode: ['server'], + skip_mode: ['server', 'async-server'], html: `
Value in child component:
diff --git a/packages/svelte/tests/runtime-legacy/shared.ts b/packages/svelte/tests/runtime-legacy/shared.ts index efd581f6ee..b8d6a990c9 100644 --- a/packages/svelte/tests/runtime-legacy/shared.ts +++ b/packages/svelte/tests/runtime-legacy/shared.ts @@ -129,7 +129,7 @@ let console_error = console.error; export function runtime_suite(runes: boolean) { return suite_with_variants( - ['dom', 'hydrate', 'ssr'], + ['dom', 'hydrate', 'ssr', 'async-ssr'], (variant, config, test_name) => { if (!async_mode && (config.skip_no_async || test_name.startsWith('async-'))) { return true; diff --git a/packages/svelte/tests/runtime-runes/samples/async-ondestroy-ordering/A.svelte b/packages/svelte/tests/runtime-runes/samples/async-ondestroy-ordering/A.svelte new file mode 100644 index 0000000000..f73e368fc8 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-ondestroy-ordering/A.svelte @@ -0,0 +1,9 @@ + + +
A
+ \ No newline at end of file diff --git a/packages/svelte/tests/runtime-runes/samples/async-ondestroy-ordering/B.svelte b/packages/svelte/tests/runtime-runes/samples/async-ondestroy-ordering/B.svelte new file mode 100644 index 0000000000..8b107601c4 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-ondestroy-ordering/B.svelte @@ -0,0 +1,9 @@ + + +
B
\ No newline at end of file diff --git a/packages/svelte/tests/runtime-runes/samples/async-ondestroy-ordering/C.svelte b/packages/svelte/tests/runtime-runes/samples/async-ondestroy-ordering/C.svelte new file mode 100644 index 0000000000..8b616d6e94 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-ondestroy-ordering/C.svelte @@ -0,0 +1,5 @@ + \ No newline at end of file diff --git a/packages/svelte/tests/runtime-runes/samples/async-ondestroy-ordering/_config.js b/packages/svelte/tests/runtime-runes/samples/async-ondestroy-ordering/_config.js new file mode 100644 index 0000000000..32def88537 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-ondestroy-ordering/_config.js @@ -0,0 +1,14 @@ +import { test } from '../../test'; +import { destroyed, reset } from './destroyed.js'; + +export default test({ + mode: ['async-server'], + + before_test() { + reset(); + }, + + test_ssr({ assert }) { + assert.deepEqual(destroyed, ['C', 'A', 'B', 'B*', 'root']); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/async-ondestroy-ordering/destroyed.js b/packages/svelte/tests/runtime-runes/samples/async-ondestroy-ordering/destroyed.js new file mode 100644 index 0000000000..1afebed3bd --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-ondestroy-ordering/destroyed.js @@ -0,0 +1,4 @@ +/** @type {string[]} */ +export const destroyed = []; + +export const reset = () => (destroyed.length = 0); diff --git a/packages/svelte/tests/runtime-runes/samples/async-ondestroy-ordering/main.svelte b/packages/svelte/tests/runtime-runes/samples/async-ondestroy-ordering/main.svelte new file mode 100644 index 0000000000..8503a54681 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/async-ondestroy-ordering/main.svelte @@ -0,0 +1,13 @@ + + +{#if await Promise.resolve(true)} + +{/if} + \ No newline at end of file diff --git a/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/server/index.svelte.js index abfc264fea..e426a08989 100644 --- a/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/server/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/class-state-field-constructor-assignment/_expected/server/index.svelte.js @@ -1,38 +1,36 @@ import * as $ from 'svelte/internal/server'; export default function Class_state_field_constructor_assignment($$payload, $$props) { - $.push(); - - class Foo { - a = 0; - #b; - #foo = $.derived(() => ({ bar: this.a * 2 })); - - get foo() { - return this.#foo(); - } - - set foo($$value) { - return this.#foo($$value); - } - - #bar = $.derived(() => ({ baz: this.foo })); - - get bar() { - return this.#bar(); - } - - set bar($$value) { - return this.#bar($$value); + $$payload.component(($$payload) => { + class Foo { + a = 0; + #b; + #foo = $.derived(() => ({ bar: this.a * 2 })); + + get foo() { + return this.#foo(); + } + + set foo($$value) { + return this.#foo($$value); + } + + #bar = $.derived(() => ({ baz: this.foo })); + + get bar() { + return this.#bar(); + } + + set bar($$value) { + return this.#bar($$value); + } + + constructor() { + this.a = 1; + this.#b = 2; + this.foo.bar = 3; + this.bar = 4; + } } - - constructor() { - this.a = 1; - this.#b = 2; - this.foo.bar = 3; - this.bar = 4; - } - } - - $.pop(); + }); } \ No newline at end of file diff --git a/packages/svelte/tests/snapshot/samples/props-identifier/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/props-identifier/_expected/server/index.svelte.js index 33a3633939..7871172433 100644 --- a/packages/svelte/tests/snapshot/samples/props-identifier/_expected/server/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/props-identifier/_expected/server/index.svelte.js @@ -1,16 +1,15 @@ import * as $ from 'svelte/internal/server'; export default function Props_identifier($$payload, $$props) { - $.push(); + $$payload.component(($$payload) => { + let { $$slots, $$events, ...props } = $$props; - let { $$slots, $$events, ...props } = $$props; - - props.a; - props[a]; - props.a.b; - props.a.b = true; - props.a = true; - props[a] = true; - props; - $.pop(); + props.a; + props[a]; + props.a.b; + props.a.b = true; + props.a = true; + props[a] = true; + props; + }); } \ No newline at end of file