diff --git a/.changeset/hot-games-tap.md b/.changeset/hot-games-tap.md new file mode 100644 index 0000000000..c87b419365 --- /dev/null +++ b/.changeset/hot-games-tap.md @@ -0,0 +1,5 @@ +--- +"svelte": patch +--- + +feat: dedupe serialized hydratable values diff --git a/packages/svelte/src/internal/client/dom/blocks/await.js b/packages/svelte/src/internal/client/dom/blocks/await.js index 194a7ccc0e..5ca2059a85 100644 --- a/packages/svelte/src/internal/client/dom/blocks/await.js +++ b/packages/svelte/src/internal/client/dom/blocks/await.js @@ -1,5 +1,5 @@ /** @import { Source, TemplateNode } from '#client' */ -import { is_promise } from '../../../shared/utils.js'; +import { is_promiselike } from '../../../shared/utils.js'; import { block } from '../../reactivity/effects.js'; import { internal_set, mutable_source, source } from '../../reactivity/sources.js'; import { @@ -59,7 +59,7 @@ export function await_block(node, get_input, pending_fn, then_fn, catch_fn) { /** Whether or not there was a hydration mismatch. Needs to be a `let` or else it isn't treeshaken out */ // @ts-ignore coercing `node` to a `Comment` causes TypeScript and Prettier to fight - let mismatch = hydrating && is_promise(input) === (node.data === HYDRATION_START_ELSE); + let mismatch = hydrating && is_promiselike(input) === (node.data === HYDRATION_START_ELSE); if (mismatch) { // Hydration mismatch: remove everything inside the anchor and start fresh @@ -67,7 +67,7 @@ export function await_block(node, get_input, pending_fn, then_fn, catch_fn) { set_hydrating(false); } - if (is_promise(input)) { + if (is_promiselike(input)) { var restore = capture(); var resolved = false; diff --git a/packages/svelte/src/internal/server/hydratable.js b/packages/svelte/src/internal/server/hydratable.js index be75288604..525c2fb55c 100644 --- a/packages/svelte/src/internal/server/hydratable.js +++ b/packages/svelte/src/internal/server/hydratable.js @@ -5,6 +5,7 @@ import * as e from './errors.js'; import * as devalue from 'devalue'; import { DEV } from 'esm-env'; import { get_user_code_location } from './dev.js'; +import { is_promise } from '../shared/utils.js'; /** * @template T @@ -23,7 +24,7 @@ export function hydratable(key, fn) { if (entry !== undefined) { if (DEV) { - const comparison = compare(key, entry, encode(key, fn())); + const comparison = compare(key, entry, encode(fn())); comparison.catch(() => {}); hydratable.comparisons.push(comparison); } @@ -33,109 +34,88 @@ export function hydratable(key, fn) { const value = fn(); - entry = encode(key, value, hydratable.unresolved_promises); + entry = encode(value); hydratable.lookup.set(key, entry); return value; } /** - * @param {string} key * @param {any} value - * @param {Map, string>} [unresolved] */ -function encode(key, value, unresolved) { +function encode(value) { /** @type {HydratableLookupEntry} */ - const entry = { value, serialized: '' }; + const entry = { value }; if (DEV) { entry.stack = get_user_code_location(); } - let uid = 1; - - entry.serialized = devalue.uneval(entry.value, (value, uneval) => { - if (is_promise(value)) { - // we serialize promises as `"${i}"`, because it's impossible for that string - // to occur 'naturally' (since the quote marks would have to be escaped) - // this placeholder is returned synchronously from `uneval`, which includes it in the - // serialized string. Later (at least one microtask from now), when `p.then` runs, it'll - // be replaced. - const placeholder = `"${uid++}"`; - const p = value - .then((v) => { - entry.serialized = entry.serialized.replace( - placeholder, - // use the function form here to prevent any string replacement characters from being interpreted - // in `v`, as it's potentially user-controlled and therefore potentially malicious. - // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/replace#specifying_a_string_as_the_replacement - () => `r(${uneval(v)})` - ); - }) - .catch((devalue_error) => - e.hydratable_serialization_failed( - key, - serialization_stack(entry.stack, devalue_error?.stack) - ) - ); - - unresolved?.set(p, key); - // prevent unhandled rejections from crashing the server, track which promises are still resolving when render is complete - p.catch(() => {}).finally(() => unresolved?.delete(p)); - - (entry.promises ??= []).push(p); - return placeholder; - } - }); - return entry; } /** - * @param {any} value - * @returns {value is Promise} - */ -function is_promise(value) { - // we use this check rather than `instanceof Promise` - // because it works cross-realm - return Object.prototype.toString.call(value) === '[object Promise]'; -} - -/** + * This function runs in development to ensure that if `hydratable` is called + * twice with the same key, both occurrences use the same value * @param {string} key * @param {HydratableLookupEntry} a * @param {HydratableLookupEntry} b */ async function compare(key, a, b) { - // note: these need to be loops (as opposed to Promise.all) because - // additional promises can get pushed to them while we're awaiting - // an earlier one - for (const p of a?.promises ?? []) { - await p; + /** + * A simplified version of the logic in `renderer.js` + * @param {any} value + */ + async function serialize(value) { + /** @type {Promise[]} */ + const promises = []; + + let uid = 1; + + let serialized = devalue.uneval(value, (value, uneval) => { + if (is_promise(value)) { + const placeholder = `"${uid++}"`; + const p = value.then((v) => { + serialized = serialized.replace(placeholder, () => uneval(v)); + }); + + promises.push(p); + return placeholder; + } + }); + + // a loop, not Promise.all, as it may change while we're awaiting + for (const p of promises) await p; + + return serialized; } - for (const p of b?.promises ?? []) { - await p; + try { + if ((await serialize(a.value)) === (await serialize(b.value))) { + return; + } + } catch { + // disregard any errors that happen during serialization, + // they will be dealt with separately + return; } - if (a.serialized !== b.serialized) { - const a_stack = /** @type {string} */ (a.stack); - const b_stack = /** @type {string} */ (b.stack); + const a_stack = /** @type {string} */ (a.stack); + const b_stack = /** @type {string} */ (b.stack); - const stack = - a_stack === b_stack - ? `Occurred at:\n${a_stack}` - : `First occurrence at:\n${a_stack}\n\nSecond occurrence at:\n${b_stack}`; + const stack = + a_stack === b_stack + ? `Occurred at:\n${a_stack}` + : `First occurrence at:\n${a_stack}\n\nSecond occurrence at:\n${b_stack}`; - e.hydratable_clobbering(key, stack); - } + e.hydratable_clobbering(key, stack); } /** * @param {string | undefined} root_stack * @param {string | undefined} uneval_stack */ -function serialization_stack(root_stack, uneval_stack) { +export function serialization_stack(root_stack, uneval_stack) { let out = ''; if (root_stack) { out += root_stack + '\n'; diff --git a/packages/svelte/src/internal/server/index.js b/packages/svelte/src/internal/server/index.js index 12f76f188e..830e0f332f 100644 --- a/packages/svelte/src/internal/server/index.js +++ b/packages/svelte/src/internal/server/index.js @@ -3,7 +3,7 @@ /** @import { Store } from '#shared' */ 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'; +import { is_promiselike, noop } from '../shared/utils.js'; import { subscribe_to_store } from '../../store/utils.js'; import { UNINITIALIZED, @@ -410,7 +410,7 @@ export function bind_props(props_parent, props_now) { * @returns {void} */ function await_block(renderer, promise, pending_fn, then_fn) { - if (is_promise(promise)) { + if (is_promiselike(promise)) { renderer.push(BLOCK_OPEN); promise.then(null, noop); if (pending_fn !== null) { diff --git a/packages/svelte/src/internal/server/render-context.js b/packages/svelte/src/internal/server/render-context.js index 7a014a1b15..365bce1918 100644 --- a/packages/svelte/src/internal/server/render-context.js +++ b/packages/svelte/src/internal/server/render-context.js @@ -31,8 +31,7 @@ export async function with_render_context(fn) { context = { hydratable: { lookup: new Map(), - comparisons: [], - unresolved_promises: new Map() + comparisons: [] } }; diff --git a/packages/svelte/src/internal/server/renderer.js b/packages/svelte/src/internal/server/renderer.js index 35aac64721..7c71e25c1e 100644 --- a/packages/svelte/src/internal/server/renderer.js +++ b/packages/svelte/src/internal/server/renderer.js @@ -1,5 +1,5 @@ /** @import { Component } from 'svelte' */ -/** @import { Csp, HydratableContext, RenderOutput, SSRContext, SyncRenderOutput, Sha256Source } from './types.js' */ +/** @import { Csp, HydratableContext, RenderOutput, SSRContext, SyncRenderOutput, Sha256Source, HydratableLookupEntry } from './types.js' */ /** @import { MaybePromise } from '#shared' */ import { async_mode_flag } from '../flags/index.js'; import { abort } from './abort-signal.js'; @@ -12,8 +12,9 @@ import { attributes } from './index.js'; import { get_render_context, with_render_context, init_render_context } from './render-context.js'; import { sha256 } from './crypto.js'; import * as devalue from 'devalue'; -import { has_own_property, noop } from '../shared/utils.js'; +import { has_own_property, is_promise, noop } from '../shared/utils.js'; import { escape_html } from '../../escaping.js'; +import { serialization_stack } from './hydratable.js'; /** @typedef {'head' | 'body'} RendererType */ /** @typedef {{ [key in RendererType]: string }} AccumulatedContent */ @@ -659,7 +660,7 @@ export class Renderer { try { const renderer = Renderer.#open_render('async', component, options); const content = await renderer.#collect_content_async(); - const hydratables = await renderer.#collect_hydratables(); + const hydratables = await renderer.#hydratable_block(); if (hydratables !== null) { content.head = hydratables + content.head; } @@ -739,23 +740,6 @@ export class Renderer { return content; } - async #collect_hydratables() { - const ctx = get_render_context().hydratable; - - for (const [_, key] of ctx.unresolved_promises) { - // this is a problem -- it means we've finished the render but we're still waiting on a promise to resolve so we can - // serialize it, so we're blocking the response on useless content. - w.unresolved_hydratable(key, ctx.lookup.get(key)?.stack ?? ''); - } - - for (const comparison of ctx.comparisons) { - // these reject if there's a mismatch - await comparison; - } - - return await this.#hydratable_block(ctx); - } - /** * @template {Record} Props * @param {'sync' | 'async'} mode @@ -821,29 +805,94 @@ export class Renderer { }; } - /** - * @param {HydratableContext} ctx - */ - async #hydratable_block(ctx) { + async #hydratable_block() { + const ctx = get_render_context().hydratable; + + for (const comparison of ctx.comparisons) { + // these reject if there's a mismatch + await comparison; + } + if (ctx.lookup.size === 0) { return null; } - let entries = []; - let has_promises = false; + /** @type {Map, string>} */ + const unresolved_promises = new Map(); + + /** @type {Promise[]} */ + const promises = []; + const entries = Array.from(ctx.lookup).map(([k, v]) => [k, v.value]); + + let uid = 1; + + /** @type {string | null} */ + let serializing = null; + + let serialized = devalue.uneval(entries, (value, uneval) => { + if (entries.includes(value)) { + serializing = value[0]; + } + + const key = /** @type {string} */ (serializing); + + if (is_promise(value)) { + // we serialize promises as `"${i}"`, because it's impossible for that string + // to occur 'naturally' (since the quote marks would have to be escaped) + // this placeholder is returned synchronously from `uneval`, which includes it in the + // serialized string. Later (at least one microtask from now), when `p.then` runs, it'll + // be replaced. + const placeholder = `"${uid++}"`; + const p = value + .then((v) => { + serialized = serialized.replace( + placeholder, + // use the function form here to prevent any string replacement characters from being interpreted + // in `v`, as it's potentially user-controlled and therefore potentially malicious. + // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/replace#specifying_a_string_as_the_replacement + () => { + serializing = key; + return `r(${uneval(v)})`; + } + ); + }) + // TODO figure out how best to report these errors + .catch((devalue_error) => { + const entry = /** @type {HydratableLookupEntry} */ (ctx.lookup.get(key)); + + e.hydratable_serialization_failed( + key, + serialization_stack(entry.stack, devalue_error?.stack) + ); + }); + + unresolved_promises.set(p, key); + + // prevent unhandled rejections from crashing the server, track which promises are still resolving when render is complete + p.catch(() => {}).finally(() => { + unresolved_promises.delete(p); + }); + + promises.push(p); + return placeholder; + } + }); - for (const [k, v] of ctx.lookup) { - if (v.promises) { - has_promises = true; - for (const p of v.promises) await p; + setTimeout(() => { + for (const key of unresolved_promises.values()) { + // this is a problem -- it means we've finished the render but we're still waiting on a promise to resolve so we can + // serialize it, so we're blocking the response on useless content. + w.unresolved_hydratable(key, ctx.lookup.get(key)?.stack ?? ''); } + }, 0); - entries.push(`[${devalue.uneval(k)},${v.serialized}]`); + for (const p of promises) { + await p; } let prelude = `const h = (window.__svelte ??= {}).h ??= new Map();`; - if (has_promises) { + if (promises.length > 0) { prelude = `const r = (v) => Promise.resolve(v); ${prelude}`; } @@ -852,9 +901,7 @@ export class Renderer { { ${prelude} - for (const [k, v] of [ - ${entries.join(',\n\t\t\t\t\t')} - ]) { + for (const [k, v] of ${serialized}) { h.set(k, v); } } diff --git a/packages/svelte/src/internal/server/types.d.ts b/packages/svelte/src/internal/server/types.d.ts index ea6282c176..08d7c80fde 100644 --- a/packages/svelte/src/internal/server/types.d.ts +++ b/packages/svelte/src/internal/server/types.d.ts @@ -19,7 +19,6 @@ export type Csp = { nonce?: string; hash?: boolean }; export interface HydratableLookupEntry { value: unknown; - serialized: string; promises?: Array>; /** dev-only */ stack?: string; @@ -28,7 +27,6 @@ export interface HydratableLookupEntry { export interface HydratableContext { lookup: Map; comparisons: Promise[]; - unresolved_promises: Map, string>; } export interface RenderContext { diff --git a/packages/svelte/src/internal/shared/utils.js b/packages/svelte/src/internal/shared/utils.js index 393b51383e..b08e5f05b6 100644 --- a/packages/svelte/src/internal/shared/utils.js +++ b/packages/svelte/src/internal/shared/utils.js @@ -32,10 +32,20 @@ export const noop = () => {}; * @param {any} value * @returns {value is PromiseLike} */ -export function is_promise(value) { +export function is_promiselike(value) { return typeof value?.then === 'function'; } +/** + * @param {any} value + * @returns {value is Promise} + */ +export function is_promise(value) { + // we use this check rather than `instanceof Promise` + // because it works cross-realm + return Object.prototype.toString.call(value) === '[object Promise]'; +} + /** @param {Function} fn */ export function run(fn) { return fn(); diff --git a/packages/svelte/tests/runtime-runes/samples/hydratable-unused-keys-nesting-partial/main.svelte b/packages/svelte/tests/runtime-runes/samples/hydratable-unused-keys-nesting-partial/main.svelte index 75723848b1..e437479418 100644 --- a/packages/svelte/tests/runtime-runes/samples/hydratable-unused-keys-nesting-partial/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/hydratable-unused-keys-nesting-partial/main.svelte @@ -11,7 +11,7 @@ (res, rej) => environment === 'server' ? setTimeout(() => res('did you ever hear the tragedy of darth plagueis the wise?'), 0) : rej('should not run') ), unused: new Promise( - (res, rej) => environment === 'server' ? setTimeout(() => res('no, sith daddy, please tell me'), 0) : rej('should not run') + (res, rej) => environment === 'server' ? setTimeout(() => res('no, sith daddy, please tell me'), 10) : rej('should not run') ), } } diff --git a/packages/svelte/tests/runtime-runes/samples/hydratable-unused-keys/main.svelte b/packages/svelte/tests/runtime-runes/samples/hydratable-unused-keys/main.svelte index 67848e7f6f..8df33a6cfe 100644 --- a/packages/svelte/tests/runtime-runes/samples/hydratable-unused-keys/main.svelte +++ b/packages/svelte/tests/runtime-runes/samples/hydratable-unused-keys/main.svelte @@ -6,7 +6,7 @@ const unresolved_hydratable = hydratable( "unused_key", () => new Promise( - (res, rej) => environment === 'server' ? setTimeout(() => res('did you ever hear the tragedy of darth plagueis the wise?'), 0) : rej('should not run') + (res, rej) => environment === 'server' ? setTimeout(() => res('did you ever hear the tragedy of darth plagueis the wise?'), 10) : rej('should not run') ) ); diff --git a/packages/svelte/tests/server-side-rendering/samples/csp-hash/_config.js b/packages/svelte/tests/server-side-rendering/samples/csp-hash/_config.js index fe28087d86..b3fb2b4b2b 100644 --- a/packages/svelte/tests/server-side-rendering/samples/csp-hash/_config.js +++ b/packages/svelte/tests/server-side-rendering/samples/csp-hash/_config.js @@ -3,5 +3,5 @@ import { test } from '../../test'; export default test({ mode: ['async'], csp: { hash: true }, - script_hashes: ['sha256-J0xwNm40i0NVEdHYeMRThG7y90X+P/I1ElZGnpQ0AbU='] + script_hashes: ['sha256-4HHPDi8jS0iIdBRKHhM8ZWr+TwaCXwZJQVYhljcWYPE='] }); diff --git a/packages/svelte/tests/server-side-rendering/samples/csp-hash/_expected_head.html b/packages/svelte/tests/server-side-rendering/samples/csp-hash/_expected_head.html index 56319255d6..6b117fff45 100644 --- a/packages/svelte/tests/server-side-rendering/samples/csp-hash/_expected_head.html +++ b/packages/svelte/tests/server-side-rendering/samples/csp-hash/_expected_head.html @@ -3,10 +3,8 @@ const r = (v) => Promise.resolve(v); const h = (window.__svelte ??= {}).h ??= new Map(); - for (const [k, v] of [ - ["key",r("bar")] - ]) { + for (const [k, v] of [["key",r("bar")]]) { h.set(k, v); } } - \ No newline at end of file + diff --git a/packages/svelte/tests/server-side-rendering/samples/csp-nonce/_expected_head.html b/packages/svelte/tests/server-side-rendering/samples/csp-nonce/_expected_head.html index fb3c95f51f..0cbcd156f4 100644 --- a/packages/svelte/tests/server-side-rendering/samples/csp-nonce/_expected_head.html +++ b/packages/svelte/tests/server-side-rendering/samples/csp-nonce/_expected_head.html @@ -3,10 +3,8 @@ const r = (v) => Promise.resolve(v); const h = (window.__svelte ??= {}).h ??= new Map(); - for (const [k, v] of [ - ["key",r("bar")] - ]) { + for (const [k, v] of [["key",r("bar")]]) { h.set(k, v); } } - \ No newline at end of file +