diff --git a/.changeset/eager-crabs-study.md b/.changeset/eager-crabs-study.md new file mode 100644 index 000000000000..9179a6d2bcac --- /dev/null +++ b/.changeset/eager-crabs-study.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': minor +--- + +breaking: stabilize remote function caching by sorting object keys diff --git a/documentation/docs/20-core-concepts/60-remote-functions.md b/documentation/docs/20-core-concepts/60-remote-functions.md index dedfc0f06e17..75fee4217169 100644 --- a/documentation/docs/20-core-concepts/60-remote-functions.md +++ b/documentation/docs/20-core-concepts/60-remote-functions.md @@ -160,6 +160,8 @@ export const getPost = query(v.string(), async (slug) => { Both the argument and the return value are serialized with [devalue](https://github.com/sveltejs/devalue), which handles types like `Date` and `Map` (and custom types defined in your [transport hook](hooks#Universal-hooks-transport)) in addition to JSON. +> [!NOTE] For `query` and `prerender` arguments (but not return values), objects, maps, and sets are sorted so that instances with the same members result in the same cache key. For example, `getPosts({ limit: 10, offset: 10 })` and `getPosts({ offset: 10, limit: 10 })` will result in the same cache key. If order is important to you, you'll have to use an array. + ### Refreshing queries Any query can be re-fetched via its `refresh` method, which retrieves the latest value from the server: diff --git a/packages/kit/src/runtime/client/remote-functions/command.svelte.js b/packages/kit/src/runtime/client/remote-functions/command.svelte.js index 5838f406e88f..660220c5fa9e 100644 --- a/packages/kit/src/runtime/client/remote-functions/command.svelte.js +++ b/packages/kit/src/runtime/client/remote-functions/command.svelte.js @@ -43,7 +43,7 @@ export function command(id) { const response = await fetch(`${base}/${app_dir}/remote/${id}`, { method: 'POST', body: JSON.stringify({ - payload: stringify_remote_arg(arg, app.hooks.transport), + payload: stringify_remote_arg(arg, app.hooks.transport, false), refreshes: updates.map((u) => u._key) }), headers diff --git a/packages/kit/src/runtime/shared.js b/packages/kit/src/runtime/shared.js index a38f9249ac7f..ebcbb5163c38 100644 --- a/packages/kit/src/runtime/shared.js +++ b/packages/kit/src/runtime/shared.js @@ -51,17 +51,215 @@ export function stringify(data, transport) { return devalue.stringify(data, encoders); } +const object_proto_names = /* @__PURE__ */ Object.getOwnPropertyNames(Object.prototype) + .sort() + .join('\0'); + +/** + * @param {unknown} thing + * @returns {thing is Record} + */ +function is_plain_object(thing) { + if (typeof thing !== 'object' || thing === null) return false; + const proto = Object.getPrototypeOf(thing); + + return ( + proto === Object.prototype || + proto === null || + Object.getPrototypeOf(proto) === null || + Object.getOwnPropertyNames(proto).sort().join('\0') === object_proto_names + ); +} + +/** + * @param {Record} value + * @param {Map} clones + */ +function to_sorted(value, clones) { + const clone = Object.getPrototypeOf(value) === null ? Object.create(null) : {}; + clones.set(value, clone); + Object.defineProperty(clone, remote_arg_marker, { value: true }); + + for (const key of Object.keys(value).sort()) { + const property = value[key]; + Object.defineProperty(clone, key, { + value: clones.get(property) ?? property, + enumerable: true, + configurable: true, + writable: true + }); + } + + return clone; +} + +// "sveltekit remote arg" +const remote_object = '__skrao'; +const remote_map = '__skram'; +const remote_set = '__skras'; +const remote_regex_guard = '__skrag'; +const remote_arg_marker = Symbol(remote_object); + +/** + * @param {Transport} transport + * @param {boolean} sort + * @param {Map} remote_arg_clones + */ +function create_remote_arg_reducers(transport, sort, remote_arg_clones) { + /** @type {Record unknown>} */ + const remote_fns_reducers = { + [remote_regex_guard]: + /** @type {(value: unknown) => void} */ + (value) => { + if (value instanceof RegExp) { + throw new Error('Regular expressions are not valid remote function arguments'); + } + } + }; + + if (sort) { + /** @type {(value: unknown) => Array<[unknown, unknown]> | undefined} */ + remote_fns_reducers[remote_map] = (value) => { + if (!(value instanceof Map)) { + return; + } + + /** @type {Array<[string, string]>} */ + const entries = []; + + for (const [key, val] of value) { + entries.push([stringify(key), stringify(val)]); + } + + return entries.sort(([a1, a2], [b1, b2]) => { + if (a1 < b1) return -1; + if (a1 > b1) return 1; + if (a2 < b2) return -1; + if (a2 > b2) return 1; + return 0; + }); + }; + + /** @type {(value: unknown) => unknown[] | undefined} */ + remote_fns_reducers[remote_set] = (value) => { + if (!(value instanceof Set)) { + return; + } + + /** @type {string[]} */ + const items = []; + + for (const item of value) { + items.push(stringify(item)); + } + + items.sort(); + return items; + }; + + /** @type {(value: unknown) => Record | undefined} */ + remote_fns_reducers[remote_object] = (value) => { + if (!is_plain_object(value)) { + return; + } + + if (Object.hasOwn(value, remote_arg_marker)) { + return; + } + + if (remote_arg_clones.has(value)) { + return remote_arg_clones.get(value); + } + + return to_sorted(value, remote_arg_clones); + }; + } + + const user_reducers = Object.fromEntries( + Object.entries(transport).map(([k, v]) => [k, v.encode]) + ); + const all_reducers = { ...user_reducers, ...remote_fns_reducers }; + + /** @type {(value: unknown) => string} */ + const stringify = (value) => devalue.stringify(value, all_reducers); + + return all_reducers; +} + +/** @param {Transport} transport */ +function create_remote_arg_revivers(transport) { + const remote_fns_revivers = { + /** @type {(value: unknown) => unknown} */ + [remote_object]: (value) => value, + /** @type {(value: unknown) => Map} */ + [remote_map]: (value) => { + if (!Array.isArray(value)) { + throw new Error('Invalid data for Map reviver'); + } + + const map = new Map(); + + for (const item of value) { + if ( + !Array.isArray(item) || + item.length !== 2 || + typeof item[0] !== 'string' || + typeof item[1] !== 'string' + ) { + throw new Error('Invalid data for Map reviver'); + } + const [key, val] = item; + map.set(parse(key), parse(val)); + } + + return map; + }, + /** @type {(value: unknown) => Set} */ + [remote_set]: (value) => { + if (!Array.isArray(value)) { + throw new Error('Invalid data for Set reviver'); + } + + const set = new Set(); + + for (const item of value) { + if (typeof item !== 'string') { + throw new Error('Invalid data for Set reviver'); + } + set.add(parse(item)); + } + + return set; + } + }; + + const user_revivers = Object.fromEntries( + Object.entries(transport).map(([k, v]) => [k, v.decode]) + ); + + const all_revivers = { ...user_revivers, ...remote_fns_revivers }; + + /** @type {(data: string) => unknown} */ + const parse = (data) => devalue.parse(data, all_revivers); + + return all_revivers; +} + /** * Stringifies the argument (if any) for a remote function in such a way that * it is both a valid URL and a valid file name (necessary for prerendering). * @param {any} value * @param {Transport} transport + * @param {boolean} [sort] */ -export function stringify_remote_arg(value, transport) { +export function stringify_remote_arg(value, transport, sort = true) { if (value === undefined) return ''; // If people hit file/url size limits, we can look into using something like compress_and_encode_text from svelte.dev beyond a certain size - const json_string = stringify(value, transport); + const json_string = devalue.stringify( + value, + create_remote_arg_reducers(transport, sort, new Map()) + ); const bytes = new TextEncoder().encode(json_string); return base64_encode(bytes).replaceAll('=', '').replaceAll('+', '-').replaceAll('/', '_'); @@ -80,9 +278,7 @@ export function parse_remote_arg(string, transport) { base64_decode(string.replaceAll('-', '+').replaceAll('_', '/')) ); - const decoders = Object.fromEntries(Object.entries(transport).map(([k, v]) => [k, v.decode])); - - return devalue.parse(json_string, decoders); + return devalue.parse(json_string, create_remote_arg_revivers(transport)); } /** diff --git a/packages/kit/src/runtime/shared.spec.js b/packages/kit/src/runtime/shared.spec.js new file mode 100644 index 000000000000..48a00e6eb6f1 --- /dev/null +++ b/packages/kit/src/runtime/shared.spec.js @@ -0,0 +1,371 @@ +import { describe, expect, test } from 'vitest'; +import { parse_remote_arg, stringify_remote_arg } from './shared.js'; + +class Thing { + /** @param {number} a @param {number} z */ + constructor(a, z) { + this.a = a; + this.z = z; + } +} + +const transport = { + Thing: { + /** @param {unknown} value */ + encode: (value) => (value instanceof Thing ? { z: value.z, a: value.a } : undefined), + /** @param {{ a: number; z: number }} value */ + decode: (value) => new Thing(value.a, value.z) + } +}; + +/** @param {Array<[any, any]>} entries */ +function map(entries) { + return /** @type {Map} */ (new Map(entries)); +} + +/** @param {any[]} items */ +function set(items) { + return /** @type {Set} */ (new Set(items)); +} + +describe('stringify_remote_arg', () => { + test('produces the same key for reordered plain object properties', () => { + const a = stringify_remote_arg({ limit: 10, offset: 20 }, {}); + const b = stringify_remote_arg({ offset: 20, limit: 10 }, {}); + + expect(a).toBe(b); + }); + + test('produces the same key for reordered nested plain object properties', () => { + const a = stringify_remote_arg( + { + filter: { + range: { min: 1, max: 5 }, + tags: ['a', 'b'] + } + }, + {} + ); + + const b = stringify_remote_arg( + { + filter: { + tags: ['a', 'b'], + range: { max: 5, min: 1 } + } + }, + {} + ); + + expect(a).toBe(b); + }); + + test('produces the same key for reordered null-prototype object properties', () => { + const a = Object.assign(Object.create(null), { limit: 10, offset: 20 }); + const b = Object.assign(Object.create(null), { offset: 20, limit: 10 }); + + expect(stringify_remote_arg(a, {})).toBe(stringify_remote_arg(b, {})); + }); + + test('produces the same key for reordered Map entries', () => { + const a = stringify_remote_arg( + map([ + [ + 'second', + map([ + ['y', { d: 4, c: 3 }], + ['x', { b: 2, a: 1 }] + ]) + ], + ['first', { nested: { z: 1, a: 2 } }] + ]), + {} + ); + + const b = stringify_remote_arg( + map([ + ['first', { nested: { a: 2, z: 1 } }], + [ + 'second', + map([ + ['x', { a: 1, b: 2 }], + ['y', { c: 3, d: 4 }] + ]) + ] + ]), + {} + ); + + expect(a).toBe(b); + }); + + test('produces the same key for reordered Set items', () => { + const a = stringify_remote_arg( + set([ + map([ + ['b', { y: 2, x: 1 }], + ['a', { b: 2, a: 1 }] + ]), + map([ + [ + 'd', + set([ + { d: 4, c: 3 }, + { b: 2, a: 1 } + ]) + ], + ['c', { z: 1, y: 2 }] + ]) + ]), + {} + ); + + const b = stringify_remote_arg( + set([ + map([ + ['c', { y: 2, z: 1 }], + [ + 'd', + set([ + { a: 1, b: 2 }, + { c: 3, d: 4 } + ]) + ] + ]), + map([ + ['a', { a: 1, b: 2 }], + ['b', { x: 1, y: 2 }] + ]) + ]), + {} + ); + + expect(a).toBe(b); + }); + + test('preserves input ordering when sort is false', () => { + const a = stringify_remote_arg({ limit: 10, offset: 20 }, {}, false); + const b = stringify_remote_arg({ offset: 20, limit: 10 }, {}, false); + + expect(a).not.toBe(b); + }); + + test('produces the same key for transported values nested inside Maps and Sets', () => { + const a = stringify_remote_arg( + map([ + ['second', set([new Thing(4, 5), new Thing(2, 3)])], + ['first', new Thing(1, 2)] + ]), + transport + ); + + const b = stringify_remote_arg( + map([ + ['first', new Thing(1, 2)], + ['second', set([new Thing(2, 3), new Thing(4, 5)])] + ]), + transport + ); + + expect(a).toBe(b); + }); + + test('does not mutate input objects while canonicalizing keys', () => { + const value = { + z: 1, + nested: { + b: 2, + a: 1 + } + }; + + stringify_remote_arg(value, {}); + + expect(Object.keys(value)).toEqual(['z', 'nested']); + expect(Object.keys(value.nested)).toEqual(['b', 'a']); + }); + + test('round-trips cycles and repeated plain-object references', () => { + const shared = { z: 1, a: 2 }; + const value = { + items: [shared, shared] + }; + // @ts-expect-error + value.self = value; + + const parsed = parse_remote_arg(stringify_remote_arg(value, {}), {}); + + expect(parsed.self).toBe(parsed); + expect(parsed.items[0]).toBe(parsed.items[1]); + expect(Object.keys(parsed.items[0])).toEqual(['a', 'z']); + }); + + test('round-trips allowed devalue builtins', () => { + const value = { + date: new Date('2024-01-01T00:00:00.000Z'), + buffer: new Uint8Array([3, 1, 2]), + url: new URL('https://example.com/?a=1') + }; + + const parsed = parse_remote_arg(stringify_remote_arg(value, {}), {}); + + expect(parsed.date.toISOString()).toBe('2024-01-01T00:00:00.000Z'); + expect(Array.from(parsed.buffer)).toEqual([3, 1, 2]); + expect(parsed.url.toString()).toBe('https://example.com/?a=1'); + }); + + test('rejects RegExp arguments', () => { + expect(() => stringify_remote_arg(/a/, {})).toThrow( + 'Regular expressions are not valid remote function arguments' + ); + }); + + test('rejects class instances via devalue', () => { + class Thing {} + + expect(() => stringify_remote_arg(new Thing(), {})).toThrow( + 'Cannot stringify arbitrary non-POJOs' + ); + }); + + test('round-trips sparse arrays while sorting nested plain objects', () => { + const value = []; + value[1_000_000] = { b: 2, a: 1 }; + + const parsed = parse_remote_arg(stringify_remote_arg(value, {}), {}); + + expect(parsed).toHaveLength(1_000_001); + expect(0 in parsed).toBe(false); + expect(Object.keys(parsed[1_000_000])).toEqual(['a', 'b']); + }); +}); + +describe('parse_remote_arg', () => { + test('returns undefined for an empty payload', () => { + expect(parse_remote_arg('', {})).toBeUndefined(); + }); + + test('parses remote-arg reducer payloads without transport decoders', () => { + const parsed = parse_remote_arg(stringify_remote_arg({ z: 1, nested: { b: 2, a: 1 } }, {}), {}); + + expect(parsed).toEqual({ nested: { a: 1, b: 2 }, z: 1 }); + }); + + test('round-trips self-referential objects', () => { + const value = { z: 1, a: 2 }; + // @ts-expect-error + value.self = value; + + const parsed = parse_remote_arg(stringify_remote_arg(value, {}), {}); + + expect(parsed.self).toBe(parsed); + expect(Object.keys(parsed)).toEqual(['a', 'self', 'z']); + }); + + test('round-trips Maps with stable ordering and nested data structures', () => { + const value = map([ + [ + 'second', + map([ + ['y', { d: 4, c: 3 }], + [ + 'x', + set([ + { d: 4, c: 3 }, + { b: 2, a: 1 } + ]) + ] + ]) + ], + ['first', { nested: { z: 1, a: 2 } }] + ]); + + const parsed = parse_remote_arg(stringify_remote_arg(value, {}), {}); + + expect(parsed).toBeInstanceOf(Map); + expect(Array.from(parsed.keys())).toEqual(['first', 'second']); + expect(Object.keys(parsed.get('first'))).toEqual(['nested']); + expect(Object.keys(parsed.get('first').nested)).toEqual(['a', 'z']); + + const nested_map = parsed.get('second'); + expect(nested_map).toBeInstanceOf(Map); + expect(Array.from(nested_map.keys())).toEqual(['x', 'y']); + expect(Array.from(nested_map.get('x'))).toEqual([ + { a: 1, b: 2 }, + { c: 3, d: 4 } + ]); + expect(nested_map.get('y')).toEqual({ c: 3, d: 4 }); + }); + + test('round-trips Sets with stable ordering and nested data structures', () => { + const value = set([ + map([ + ['b', { y: 2, x: 1 }], + ['a', { b: 2, a: 1 }] + ]), + map([ + [ + 'd', + set([ + { d: 4, c: 3 }, + { b: 2, a: 1 } + ]) + ], + ['c', { z: 1, y: 2 }] + ]) + ]); + + const parsed = parse_remote_arg(stringify_remote_arg(value, {}), {}); + + expect(parsed).toBeInstanceOf(Set); + + const [first, second] = Array.from(parsed); + expect(first).toBeInstanceOf(Map); + expect(Array.from(first.keys())).toEqual(['a', 'b']); + expect(first.get('a')).toEqual({ a: 1, b: 2 }); + expect(first.get('b')).toEqual({ x: 1, y: 2 }); + + expect(second).toBeInstanceOf(Map); + expect(Array.from(second.keys())).toEqual(['c', 'd']); + expect(second.get('c')).toEqual({ y: 2, z: 1 }); + expect(Array.from(second.get('d'))).toEqual([ + { a: 1, b: 2 }, + { c: 3, d: 4 } + ]); + }); + + test('round-trips transport values nested inside Maps and Sets', () => { + const value = map([ + ['second', set([new Thing(4, 5), new Thing(2, 3)])], + ['first', new Thing(1, 2)] + ]); + + const parsed = parse_remote_arg(stringify_remote_arg(value, transport), transport); + + expect(parsed).toBeInstanceOf(Map); + expect(Array.from(parsed.keys())).toEqual(['first', 'second']); + expect(parsed.get('first')).toBeInstanceOf(Thing); + expect(parsed.get('first')).toMatchObject({ a: 1, z: 2 }); + + const nested = parsed.get('second'); + expect(nested).toBeInstanceOf(Set); + expect(Array.from(nested)).toEqual([ + expect.objectContaining({ a: 2, z: 3 }), + expect.objectContaining({ a: 4, z: 5 }) + ]); + }); + + test('restores null-prototype objects', () => { + const value = Object.assign(Object.create(null), { + z: 1, + nested: Object.assign(Object.create(null), { b: 2, a: 1 }) + }); + + const parsed = parse_remote_arg(stringify_remote_arg(value, {}), {}); + + expect(Object.getPrototypeOf(parsed)).toBeNull(); + expect(Object.getPrototypeOf(parsed.nested)).toBeNull(); + expect(Object.keys(parsed)).toEqual(['nested', 'z']); + expect(Object.keys(parsed.nested)).toEqual(['a', 'b']); + }); +});