From fba1eaf4e35db06630ab8c868b91cd290d6d6092 Mon Sep 17 00:00:00 2001 From: ljodea Date: Wed, 29 Jul 2026 17:12:11 -0500 Subject: [PATCH] perf: O(1) key membership in spread_props ownKeys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Building unique keys for Reflect.ownKeys on spread props used Array.includes while growing the list (O(k²) in unique key count). Use a Set so membership is O(1) while keeping first-seen order and the same for...in / getOwnPropertySymbols semantics. Closes #18600 --- .changeset/spread-props-ownkeys-set.md | 5 ++ .../src/internal/client/reactivity/props.js | 10 +-- .../internal/client/reactivity/props.test.ts | 68 +++++++++++++++++++ 3 files changed, 78 insertions(+), 5 deletions(-) create mode 100644 .changeset/spread-props-ownkeys-set.md create mode 100644 packages/svelte/src/internal/client/reactivity/props.test.ts diff --git a/.changeset/spread-props-ownkeys-set.md b/.changeset/spread-props-ownkeys-set.md new file mode 100644 index 0000000000..2f625a4e2c --- /dev/null +++ b/.changeset/spread-props-ownkeys-set.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +perf: O(1) key membership in `spread_props` `ownKeys` diff --git a/packages/svelte/src/internal/client/reactivity/props.js b/packages/svelte/src/internal/client/reactivity/props.js index 274d780b1e..d310c68414 100644 --- a/packages/svelte/src/internal/client/reactivity/props.js +++ b/packages/svelte/src/internal/client/reactivity/props.js @@ -235,23 +235,23 @@ const spread_props_handler = { return false; }, ownKeys(target) { - /** @type {Array} */ - const keys = []; + /** @type {Set} */ + const keys = new Set(); for (let p of target.props) { if (is_function(p)) p = p(); if (!p) continue; for (const key in p) { - if (!keys.includes(key)) keys.push(key); + keys.add(key); } for (const key of Object.getOwnPropertySymbols(p)) { - if (!keys.includes(key)) keys.push(key); + keys.add(key); } } - return keys; + return Array.from(keys); } }; diff --git a/packages/svelte/src/internal/client/reactivity/props.test.ts b/packages/svelte/src/internal/client/reactivity/props.test.ts new file mode 100644 index 0000000000..77f85ff9df --- /dev/null +++ b/packages/svelte/src/internal/client/reactivity/props.test.ts @@ -0,0 +1,68 @@ +import { assert, describe, test } from 'vitest'; +import { spread_props } from './props.js'; + +/** + * Behavior lock for spread_props ownKeys (perf #18600). + * Constrains rewrites: first-seen order, for...in (incl. inherited enumerable strings), + * getOwnPropertySymbols (incl. non-enumerable), lazy fn props, falsy skip. + */ +describe('spread_props ownKeys', () => { + test('dedups string keys with first-seen order across objects', () => { + const props = spread_props({ z: 1, a: 2 }, { a: 3, b: 4 }); + + assert.deepEqual(Reflect.ownKeys(props), ['z', 'a', 'b']); + assert.deepEqual(Object.keys(props), ['z', 'a', 'b']); + }); + + test('integer-like key order is per-object, not global', () => { + // Object key order: integer indices ascending, then other strings in insertion order. + // Across spreads, later objects append only unseen keys — no global re-sort. + const props = spread_props({ b: 1, 2: 0, 1: 0 }, { a: 1 }); + + assert.deepEqual(Reflect.ownKeys(props), ['1', '2', 'b', 'a']); + }); + + test('includes inherited enumerable string keys from for...in', () => { + const child = Object.create( + { inherited: 1 }, + { own: { value: 1, enumerable: true, configurable: true, writable: true } } + ); + const props = spread_props(child); + + // ownKeys uses for...in → inherited enumerable strings appear + assert.deepEqual(Reflect.ownKeys(props), ['own', 'inherited']); + // Object.keys also consults getOwnPropertyDescriptor; inherited has no own descriptor + assert.deepEqual(Object.keys(props), ['own']); + }); + + test('dedups symbols at first-seen index; non-enumerable own symbols included', () => { + const s1 = Symbol('s1'); + const s2 = Symbol('s2'); + const s_ne = Symbol('non-enumerable'); + + const first: Record = { z: 1, a: 2, [s1]: 1 }; + Object.defineProperty(first, s_ne, { value: 1, enumerable: false }); + + const second: Record = { a: 3, b: 4, [s1]: 9, [s2]: 2 }; + + const props = spread_props(first, second); + + // Per object: for...in strings, then getOwnPropertySymbols (all own symbols) + assert.deepEqual(Reflect.ownKeys(props), ['z', 'a', s1, s_ne, 'b', s2]); + assert.deepEqual(Object.keys(props), ['z', 'a', 'b']); + }); + + test('evaluates lazy function props and skips nullish returns', () => { + assert.deepEqual( + Reflect.ownKeys(spread_props(() => ({ x: 1 }), { y: 2 })), + ['x', 'y'] + ); + assert.deepEqual(Reflect.ownKeys(spread_props(() => null, { a: 1 })), ['a']); + }); + + test('skips falsy non-object props (same as if (!p) continue)', () => { + // Note: has trap uses `p != null`; ownKeys uses truthiness. Harmonizing is out of scope. + // @ts-expect-error intentional falsy / nullish spreads to lock current skip semantics + assert.deepEqual(Reflect.ownKeys(spread_props(0, '', null, undefined, { a: 1 })), ['a']); + }); +});