From 0e5eeb7e453dfc4c1b0916118e132e7eb789570e Mon Sep 17 00:00:00 2001 From: "S. Elliott Johnson" Date: Fri, 12 Sep 2025 17:30:54 -0600 Subject: [PATCH] checkpoint --- .../server/visitors/RegularElement.js | 14 +-- .../server/visitors/TitleElement.js | 22 ++--- .../server/visitors/shared/utils.js | 27 ++--- .../svelte/src/compiler/types/template.d.ts | 2 + packages/svelte/src/internal/server/index.js | 72 ++++++++++++-- .../svelte/src/internal/server/payload.js | 56 ++++++++--- .../src/internal/server/payload.test.ts | 98 +++++++++++++++++++ .../_expected.html | 3 + .../main.svelte | 3 + .../_expected.html | 3 + .../main.svelte | 3 + .../_expected/server/index.svelte.js | 6 +- .../_expected/server/index.svelte.js | 3 +- .../_expected/server/index.svelte.js | 4 +- .../_expected/server/index.svelte.js | 4 +- playgrounds/sandbox/package.json | 3 +- 16 files changed, 255 insertions(+), 68 deletions(-) create mode 100644 packages/svelte/tests/server-side-rendering/samples/async-option-implicit-complex-value/_expected.html create mode 100644 packages/svelte/tests/server-side-rendering/samples/async-option-implicit-complex-value/main.svelte create mode 100644 packages/svelte/tests/server-side-rendering/samples/async-option-implicit-simple-value/_expected.html create mode 100644 packages/svelte/tests/server-side-rendering/samples/async-option-implicit-simple-value/main.svelte diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/RegularElement.js index fd99a61b17..a6ba1cb65e 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/RegularElement.js @@ -161,7 +161,10 @@ export function RegularElement(node, context) { b.call( '$.simple_valueless_option', b.id('$$payload'), - node.metadata.synthetic_value_node.expression + b.thunk( + node.metadata.synthetic_value_node.expression, + node.metadata.synthetic_value_node.metadata.expression.has_await + ) ) ) ); @@ -216,16 +219,9 @@ export function RegularElement(node, context) { // in an async world, we could technically have two adjacent select elements with async children, in which case // the second element's select_value would override the first element's select_value if the children of the first // element hadn't resolved prior to hitting the second element. - // TODO is this cast safe? const elements = state.template.splice(template_start, Infinity); state.template.push( - call_child_payload( - b.block(build_template(elements)), - // TODO this will always produce correct results (because it will produce an async function if the surrounding component is async) - // but it will false-positive and create unnecessary async functions (eg. when the component is async but the select element is not) - // we could probably optimize by checking if the select element is async. Might be worth it. - select_with_value_async - ) + call_child_payload(b.block(build_template(elements)), select_with_value_async) ); } diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/TitleElement.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/TitleElement.js index 531a7bf7d3..f335a532c3 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/TitleElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/TitleElement.js @@ -11,7 +11,7 @@ export function TitleElement(node, context) { if (node.fragment.metadata.hoisted_promises.promises.length > 0) { context.state.init.push( b.const( - node.fragment.metadata.hoisted_promises.name, + node.fragment.metadata.hoisted_promises.id, b.array(node.fragment.metadata.hoisted_promises.promises) ) ); @@ -23,20 +23,12 @@ export function TitleElement(node, context) { template.push(b.literal('')); context.state.init.push( - call_child_payload( - b.block([ - b.const('path', b.call('$$payload.get_path')), - b.let('title'), - ...build_template(template, b.id('title'), '='), - b.stmt( - b.assignment( - '=', - b.id('$$payload.global.head.title'), - b.object([b.init('path', b.id('path')), b.init('value', b.id('title'))]) - ) - ) - ]), - node.metadata.has_await + b.stmt( + b.call( + '$.build_title', + b.id('$$payload'), + b.thunk(b.block(build_template(template)), node.metadata.has_await) + ) ) ); } 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 ccb42fa02a..f9601e7878 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 @@ -32,6 +32,10 @@ export function process_children(nodes, { visit, state }) { let sequence = []; function flush() { + if (sequence.length === 0) { + return; + } + let quasi = b.quasi('', false); const quasis = [quasi]; @@ -63,26 +67,25 @@ export function process_children(nodes, { visit, state }) { } state.template.push(b.template(quasis, expressions)); + sequence = []; } - for (let i = 0; i < nodes.length; i += 1) { - const node = nodes[i]; - - if (node.type === 'Text' || node.type === 'Comment' || node.type === 'ExpressionTag') { + for (const node of nodes) { + if (node.type === 'ExpressionTag' && node.metadata.expression.has_await) { + flush(); + const visited = /** @type {Expression} */ (visit(node.expression)); + state.template.push( + b.stmt(b.call('$$payload.push', b.thunk(b.call('$.escape', visited), true))) + ); + } else if (node.type === 'Text' || node.type === 'Comment' || node.type === 'ExpressionTag') { sequence.push(node); } else { - if (sequence.length > 0) { - flush(); - sequence = []; - } - + flush(); visit(node, { ...state }); } } - if (sequence.length > 0) { - flush(); - } + flush(); } /** diff --git a/packages/svelte/src/compiler/types/template.d.ts b/packages/svelte/src/compiler/types/template.d.ts index cded34688a..9b1a3dba8f 100644 --- a/packages/svelte/src/compiler/types/template.d.ts +++ b/packages/svelte/src/compiler/types/template.d.ts @@ -57,6 +57,8 @@ export namespace AST { */ dynamic: boolean; has_await: boolean; + /** TODO document */ + is_async: boolean; hoisted_promises: { id: Identifier; promises: Expression[] }; }; } diff --git a/packages/svelte/src/internal/server/index.js b/packages/svelte/src/internal/server/index.js index 44cafd9db8..b14178c08b 100644 --- a/packages/svelte/src/internal/server/index.js +++ b/packages/svelte/src/internal/server/index.js @@ -1,6 +1,7 @@ /** @import { ComponentType, SvelteComponent } from 'svelte' */ /** @import { Component, RenderOutput } from '#server' */ /** @import { Store } from '#shared' */ +/** @import { AccumulatedContent } from './payload.js' */ export { FILENAME, HMR } from '../../constants.js'; import { attr, clsx, to_class, to_style } from '../shared/attributes.js'; import { is_promise, noop } from '../shared/utils.js'; @@ -679,15 +680,68 @@ export function valueless_option(payload, children) { * we don't have to do all of the same parsing nonsense. It also means we can avoid * coercing everything to a string. * @param {Payload} payload - * @param {unknown} child_value + * @param {(() => unknown)} child */ -export function simple_valueless_option(payload, child_value) { - if (child_value === payload.local.select_value) { - payload.compact({ - start: payload.length - 1, - fn: (content) => ({ body: content.body.slice(0, -1) + ' selected>', head: content.head }) - }); - } +export function simple_valueless_option(payload, child) { + const result = child(); + + /** + * @param {AccumulatedContent} content + * @param {unknown} child_value + * @returns {AccumulatedContent} + */ + const mark_selected = (content, child_value) => { + if (child_value === payload.local.select_value) { + return { body: content.body.slice(0, -1) + ' selected>', head: content.head }; + } + return content; + }; + + payload.compact({ + start: payload.length - 1, + fn: (content) => { + if (result instanceof Promise) { + return result.then((child_value) => mark_selected(content, child_value)); + } + return mark_selected(content, result); + } + }); + + payload.child((child_payload) => { + if (result instanceof Promise) { + return result.then((child_value) => { + child_payload.push(escape_html(child_value)); + }); + } + child_payload.push(escape_html(result)); + }); +} - payload.push(escape_html(child_value)); +/** + * Since your document can only have one `title`, we have to have some sort of algorithm for determining + * which one "wins". To do this, we perform a depth-first comparison of where the title was encountered -- + * later ones "win" over earlier ones, regardless of what order the promises resolve in. To accomodate this, we: + * - Figure out where we are in the content tree (`get_path`) + * - Render the title in its own child so that it has a defined "slot" in the payload + * - Compact that spot so that we get the entire rendered contents of the title + * - Attempt to set the global title (this is where the "wins" logic based on the path happens) + * + * TODO we could optimize this by not even rendering the title if the path wouldn't be accepted + * + * @param {Payload} payload + * @param {((payload: Payload) => void | Promise)} children + */ +export function build_title(payload, children) { + const path = payload.get_path(); + const i = payload.length; + payload.child(children); + payload.compact({ + start: i, + fn: ({ head }) => { + payload.global.head.title = { path, value: head }; + // since we can only ever render the title in this chunk, and title rendering is handled specially, + // we can just ditch the results after we've saved them globally + return { head: '', body: '' }; + } + }); } diff --git a/packages/svelte/src/internal/server/payload.js b/packages/svelte/src/internal/server/payload.js index c259bdf365..57b2eef6c3 100644 --- a/packages/svelte/src/internal/server/payload.js +++ b/packages/svelte/src/internal/server/payload.js @@ -5,6 +5,9 @@ * @template T * @typedef {T | Promise} MaybePromise */ +/** + * @typedef {string | Payload | Promise} PayloadItem + */ /** * Payloads are basically a tree of `string | Payload`s, where each `Payload` in the tree represents @@ -18,7 +21,7 @@ export class Payload { /** * The contents of the payload. - * @type {(string | Payload)[]} + * @type {PayloadItem[]} */ #out = []; @@ -50,6 +53,9 @@ export class Payload { /** * State that is local to the branch it is declared in. * It will be shallow-copied to all children. + * + * TODO I think this needs to be async-compatible if we don't want waterfall-y options but I'm willing + * to live with it for now * @type {{ select_value: string | undefined }} */ local; @@ -98,16 +104,20 @@ export class Payload { } /** - * @param {string} content + * @param {string | (() => Promise)} content */ push(content) { - this.#out.push(content); + if (typeof content === 'function') { + this.#out.push(content()); + } else { + this.#out.push(content); + } } /** * Compact everything between `start` and `end` into a single payload, then call `fn` with the result of that payload. * The compacted payload will be sync if all of the children are sync and {@link fn} is sync, otherwise it will be async. - * @param {{ start: number, end?: number, fn: (content: AccumulatedContent) => AccumulatedContent }} args + * @param {{ start: number, end?: number, fn: (content: AccumulatedContent) => AccumulatedContent | Promise }} args */ compact({ start, end = this.#out.length, fn }) { const child = new Payload(this.global, this.local, this); @@ -122,7 +132,15 @@ export class Payload { ); this.promises.followup.push(followup); } else { - Payload.#push_accumulated_content(child, fn(content)); + const transformed_content = fn(content); + if (transformed_content instanceof Promise) { + const followup = transformed_content.then((content) => + Payload.#push_accumulated_content(child, content) + ); + this.promises.followup.push(followup); + } else { + Payload.#push_accumulated_content(child, transformed_content); + } } } @@ -153,7 +171,7 @@ export class Payload { copy() { const copy = new Payload(this.global, this.local, this.parent, this.type); - copy.#out = this.#out.map((item) => (typeof item === 'string' ? item : item.copy())); + copy.#out = this.#out.map((item) => (item instanceof Payload ? item.copy() : item)); copy.promises = this.promises; return copy; } @@ -170,7 +188,7 @@ export class Payload { this.global.subsume(other.global); this.local = other.local; this.#out = other.#out.map((item) => { - if (typeof item !== 'string') { + if (item instanceof Payload) { item.subsume(item); } return item; @@ -185,7 +203,7 @@ export class Payload { /** * Collect all of the code from the `out` array and return it as a string, or a promise resolving to a string. - * @param {(string | Payload)[]} items + * @param {PayloadItem[]} items * @param {PayloadType} current_type * @param {AccumulatedContent} content * @returns {MaybePromise} @@ -208,13 +226,20 @@ export class Payload { } else { flush(); - if (item.promises.initial || item.promises.followup.length) { + if (item instanceof Promise) { has_async = true; segments.push( - Payload.#collect_content_async([item], current_type, { head: '', body: '' }) + item.then((resolved) => { + const content = { head: '', body: '' }; + content[current_type] = resolved; + return content; + }) ); + } else if (item.promises.initial || item.promises.followup.length) { + has_async = true; + segments.push(Payload.#collect_content_async([item], current_type)); } else { - const sub = Payload.#collect_content(item.#out, item.type, { head: '', body: '' }); + const sub = Payload.#collect_content(item.#out, item.type); if (sub instanceof Promise) { has_async = true; } @@ -237,16 +262,15 @@ export class Payload { /** * Collect all of the code from the `out` array and return it as a string. - * @param {(string | Payload)[]} items + * @param {PayloadItem[]} items * @param {PayloadType} current_type * @param {AccumulatedContent} content * @returns {Promise} */ 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 (typeof item === 'string') { - content[current_type] += item; - } else { + if (item instanceof Payload) { 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. @@ -257,6 +281,8 @@ 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 8407f0a4b0..af8e0f52c2 100644 --- a/packages/svelte/src/internal/server/payload.test.ts +++ b/packages/svelte/src/internal/server/payload.test.ts @@ -251,3 +251,101 @@ test('TreeHeadState title ordering favors later lexicographic paths', () => { head.title = { path: [2], value: 'F' }; assert.equal(head.title.value, 'E'); }); + +test('push accepts async functions in async context', async () => { + const payload = new Payload(new TreeState('async')); + payload.push('a'); + payload.push(async () => { + await Promise.resolve(); + return 'b'; + }); + payload.push('c'); + + const { head, body } = await payload; + assert.equal(head, ''); + assert.equal(body, 'abc'); +}); + +test('push handles async functions with different timing', async () => { + const payload = new Payload(new TreeState('async')); + + // Fast async function + payload.push(async () => { + await Promise.resolve(); + return 'fast'; + }); + + // Slow async function + payload.push(async () => { + await new Promise((resolve) => setTimeout(resolve, 10)); + return 'slow'; + }); + + // Regular string + payload.push('sync'); + + const { head, body } = await payload; + assert.equal(head, ''); + assert.equal(body, 'fastslowsync'); +}); + +test('push async functions work with head content type', async () => { + const payload = new Payload(new TreeState('async'), undefined, undefined, 'head'); + payload.push(async () => { + await Promise.resolve(); + return 'Async Title'; + }); + + const { head, body } = await payload; + assert.equal(body, ''); + assert.equal(head, 'Async Title'); +}); + +test('push async functions can be mixed with child payloads', async () => { + const payload = new Payload(new TreeState('async')); + payload.push('start-'); + + payload.push(async () => { + await Promise.resolve(); + return 'async-'; + }); + + payload.child(($$payload) => { + $$payload.push('child-'); + }); + + payload.push('-end'); + + const { head, body } = await payload; + assert.equal(head, ''); + assert.equal(body, 'start-async-child--end'); +}); + +test('push async functions work with compact operations', async () => { + const payload = new Payload(new TreeState('async')); + payload.push('a'); + payload.push(async () => { + await Promise.resolve(); + return 'b'; + }); + payload.push('c'); + + payload.compact({ + start: 0, + fn: (content) => ({ head: '', body: content.body.toUpperCase() }) + }); + + const { head, body } = await payload; + assert.equal(head, ''); + assert.equal(body, 'ABC'); +}); + +test('push async functions are not supported in sync context', () => { + const payload = new Payload(new TreeState('sync')); + payload.push('a'); + + expect(() => { + payload.push(() => Promise.resolve('b')); + payload.collect(); + }).toThrow(); +}); diff --git a/packages/svelte/tests/server-side-rendering/samples/async-option-implicit-complex-value/_expected.html b/packages/svelte/tests/server-side-rendering/samples/async-option-implicit-complex-value/_expected.html new file mode 100644 index 0000000000..f2fd8e71b9 --- /dev/null +++ b/packages/svelte/tests/server-side-rendering/samples/async-option-implicit-complex-value/_expected.html @@ -0,0 +1,3 @@ + \ No newline at end of file diff --git a/packages/svelte/tests/server-side-rendering/samples/async-option-implicit-complex-value/main.svelte b/packages/svelte/tests/server-side-rendering/samples/async-option-implicit-complex-value/main.svelte new file mode 100644 index 0000000000..2ad9b64509 --- /dev/null +++ b/packages/svelte/tests/server-side-rendering/samples/async-option-implicit-complex-value/main.svelte @@ -0,0 +1,3 @@ + \ No newline at end of file diff --git a/packages/svelte/tests/server-side-rendering/samples/async-option-implicit-simple-value/_expected.html b/packages/svelte/tests/server-side-rendering/samples/async-option-implicit-simple-value/_expected.html new file mode 100644 index 0000000000..f642109218 --- /dev/null +++ b/packages/svelte/tests/server-side-rendering/samples/async-option-implicit-simple-value/_expected.html @@ -0,0 +1,3 @@ + \ No newline at end of file diff --git a/packages/svelte/tests/server-side-rendering/samples/async-option-implicit-simple-value/main.svelte b/packages/svelte/tests/server-side-rendering/samples/async-option-implicit-simple-value/main.svelte new file mode 100644 index 0000000000..a5d91424cb --- /dev/null +++ b/packages/svelte/tests/server-side-rendering/samples/async-option-implicit-simple-value/main.svelte @@ -0,0 +1,3 @@ + \ No newline at end of file diff --git a/packages/svelte/tests/snapshot/samples/async-each-fallback-hoisting/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/async-each-fallback-hoisting/_expected/server/index.svelte.js index d497aefa0d..80c37a8d78 100644 --- a/packages/svelte/tests/snapshot/samples/async-each-fallback-hoisting/_expected/server/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/async-each-fallback-hoisting/_expected/server/index.svelte.js @@ -11,14 +11,16 @@ export default function Async_each_fallback_hoisting($$payload) { let item = each_array[$$index]; $$payload.child(async ($$payload) => { - $$payload.push(`${$.escape(await Promise.reject('This should never be reached'))}`); + $$payload.push(``); + $$payload.push(async () => $.escape(await Promise.reject('This should never be reached'))); }); } } else { $$payload.push(''); $$payload.child(async ($$payload) => { - $$payload.push(`${$.escape(await Promise.resolve(4))}`); + $$payload.push(``); + $$payload.push(async () => $.escape(await Promise.resolve(4))); }); } diff --git a/packages/svelte/tests/snapshot/samples/async-each-hoisting/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/async-each-hoisting/_expected/server/index.svelte.js index a2223cf87f..9e3e81f4f2 100644 --- a/packages/svelte/tests/snapshot/samples/async-each-hoisting/_expected/server/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/async-each-hoisting/_expected/server/index.svelte.js @@ -14,7 +14,8 @@ export default function Async_each_hoisting($$payload) { let item = each_array[$$index]; $$payload.child(async ($$payload) => { - $$payload.push(`${$.escape(await item)}`); + $$payload.push(``); + $$payload.push(async () => $.escape(await item)); }); } diff --git a/packages/svelte/tests/snapshot/samples/async-if-alternate-hoisting/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/async-if-alternate-hoisting/_expected/server/index.svelte.js index d297d63202..92941fc87c 100644 --- a/packages/svelte/tests/snapshot/samples/async-if-alternate-hoisting/_expected/server/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/async-if-alternate-hoisting/_expected/server/index.svelte.js @@ -6,13 +6,13 @@ export default function Async_if_alternate_hoisting($$payload) { $$payload.push(''); $$payload.child(async ($$payload) => { - $$payload.push(`${$.escape(await Promise.reject('no no no'))}`); + $$payload.push(async () => $.escape(await Promise.reject('no no no'))); }); } else { $$payload.push(''); $$payload.child(async ($$payload) => { - $$payload.push(`${$.escape(await Promise.resolve('yes yes yes'))}`); + $$payload.push(async () => $.escape(await Promise.resolve('yes yes yes'))); }); } diff --git a/packages/svelte/tests/snapshot/samples/async-if-hoisting/_expected/server/index.svelte.js b/packages/svelte/tests/snapshot/samples/async-if-hoisting/_expected/server/index.svelte.js index 6ec5ff92a0..83da0a2711 100644 --- a/packages/svelte/tests/snapshot/samples/async-if-hoisting/_expected/server/index.svelte.js +++ b/packages/svelte/tests/snapshot/samples/async-if-hoisting/_expected/server/index.svelte.js @@ -6,13 +6,13 @@ export default function Async_if_hoisting($$payload) { $$payload.push(''); $$payload.child(async ($$payload) => { - $$payload.push(`${$.escape(await Promise.resolve('yes yes yes'))}`); + $$payload.push(async () => $.escape(await Promise.resolve('yes yes yes'))); }); } else { $$payload.push(''); $$payload.child(async ($$payload) => { - $$payload.push(`${$.escape(await Promise.reject('no no no'))}`); + $$payload.push(async () => $.escape(await Promise.reject('no no no'))); }); } diff --git a/playgrounds/sandbox/package.json b/playgrounds/sandbox/package.json index 8100084832..a5a28879e0 100644 --- a/playgrounds/sandbox/package.json +++ b/playgrounds/sandbox/package.json @@ -12,7 +12,8 @@ "preview": "vite preview", "download": "node scripts/download.js", "hash": "node scripts/hash.js", - "create-test": "node scripts/create-test.js" + "create-test": "node scripts/create-test.js", + "start": "node run.js" }, "devDependencies": { "@sveltejs/vite-plugin-svelte": "^4.0.0-next.6",