Skip to content

Commit e99ad16

Browse files
authored
perf(shared): skip defineProperty for non-inherited keys and make clone cycle-safe (#1994)
`set`, `clone`, and `mergeTwoLevels` in `@orpc/shared` wrote every property through `Object.defineProperty`, which is several times slower than a plain assignment. Assignment creates the same own property for every key except `__proto__`, so the helper now assigns directly and only uses `Object.defineProperty` for that key. The helper is renamed from `defineOwnProperty` to `setOwn` to pair with `getOwn`. `clone` also now survives circular input instead of overflowing the stack. ## Performance - Building an 8-key object two million times drops from about 4 s to about 0.5 s, roughly 8x faster. - The common path is one string comparison followed by a plain write. ## Safety - Prototype pollution guards are unchanged: the walk in `set` still descends only into own properties, so `constructor.prototype` and `__proto__` paths never reach `Object.prototype`. - `__proto__` is the only accessor on `Object.prototype`, so it is the only key where assignment and an own property differ. It still lands as an own data property instead of re-parenting the object. ## Fixes - `clone` no longer recurses forever on self-referencing arrays or objects. Cycles become cycles in the copy, and a value referenced from several places is cloned once and stays shared. ## Testing - New tests: `clone` on circular and shared-reference input. - Shared object tests plus the callers in contract, trpc, and server pass; eslint and a type check of `packages/shared` including test files are clean.
1 parent d637f30 commit e99ad16

2 files changed

Lines changed: 84 additions & 15 deletions

File tree

packages/shared/src/object.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -412,6 +412,41 @@ describe('clone', () => {
412412
// eslint-disable-next-line no-restricted-properties, no-proto
413413
expect(clonedNullProto.__proto__).toBe(2)
414414
})
415+
416+
it('clone with circular references', () => {
417+
const obj: Record<string, unknown> = { a: 1 }
418+
obj.self = obj
419+
obj.list = [obj]
420+
421+
const cloned = clone(obj)
422+
423+
expect(cloned).not.toBe(obj)
424+
expect(cloned.a).toBe(1)
425+
expect(cloned.self).toBe(cloned)
426+
expect((cloned.list as unknown[])[0]).toBe(cloned)
427+
expect(cloned.list).not.toBe(obj.list)
428+
429+
const arr: unknown[] = [1]
430+
arr.push(arr)
431+
432+
const clonedArr = clone(arr)
433+
434+
expect(clonedArr).not.toBe(arr)
435+
expect(clonedArr[0]).toBe(1)
436+
expect(clonedArr[1]).toBe(clonedArr)
437+
})
438+
439+
it('clone keeps shared references shared', () => {
440+
const shared = { x: 1 }
441+
const obj = { a: shared, b: shared, c: [shared] }
442+
443+
const cloned = clone(obj)
444+
445+
expect(cloned.a).toEqual(shared)
446+
expect(cloned.a).not.toBe(shared)
447+
expect(cloned.b).toBe(cloned.a)
448+
expect(cloned.c[0]).toBe(cloned.a)
449+
})
415450
})
416451

417452
describe('bindMethods', () => {

packages/shared/src/object.ts

Lines changed: 49 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -107,24 +107,32 @@ export function set(
107107

108108
if (!isTypescriptObject(next)) {
109109
const child = {}
110-
defineOwnProperty(current, key, child)
110+
setOwn(current, key, child)
111111
current = child
112112
}
113113
else {
114114
current = next
115115
}
116116
}
117117

118-
defineOwnProperty(current, path.at(-1)!, value)
118+
setOwn(current, path.at(-1)!, value)
119119
}
120120

121-
function defineOwnProperty(object: object, key: PropertyKey, value: unknown): void {
122-
Object.defineProperty(object, key, {
123-
value,
124-
writable: true,
125-
enumerable: true,
126-
configurable: true,
127-
})
121+
/**
122+
* Sets `object[key]`, defining `__proto__` as an own property instead of re-parenting the object.
123+
*/
124+
function setOwn(object: object, key: PropertyKey, value: unknown): void {
125+
if (key === '__proto__') {
126+
Object.defineProperty(object, key, {
127+
value,
128+
writable: true,
129+
enumerable: true,
130+
configurable: true,
131+
})
132+
}
133+
else {
134+
(object as Record<PropertyKey, unknown>)[key] = value
135+
}
128136
}
129137

130138
/**
@@ -148,7 +156,7 @@ export function mergeTwoLevels(first: unknown, second: unknown): unknown {
148156
const secondValue = second[key]
149157

150158
if (isPlainObject(firstValue) && isPlainObject(secondValue)) {
151-
defineOwnProperty(result, key, { ...firstValue, ...secondValue })
159+
setOwn(result, key, { ...firstValue, ...secondValue })
152160
}
153161
}
154162

@@ -168,24 +176,50 @@ export function omit<T extends object, K extends keyof T>(
168176
return result
169177
}
170178

179+
/**
180+
* Deep clones arrays and plain objects, leaving every other value as is.
181+
* Circular and shared references are preserved in the copy.
182+
*/
171183
export function clone<T>(value: T): T {
184+
return cloneWithVisited(value, new WeakMap()) as T
185+
}
186+
187+
function cloneWithVisited(value: unknown, visited: WeakMap<object, unknown>): unknown {
172188
if (Array.isArray(value)) {
173-
return value.map(clone) as any
189+
const existing = visited.get(value)
190+
if (existing) {
191+
return existing
192+
}
193+
194+
const result: unknown[] = []
195+
visited.set(value, result)
196+
197+
for (const item of value) {
198+
result.push(cloneWithVisited(item, visited))
199+
}
200+
201+
return result
174202
}
175203

176204
if (isPlainObject(value)) {
205+
const existing = visited.get(value)
206+
if (existing) {
207+
return existing
208+
}
209+
177210
const result: Record<PropertyKey, unknown> = {}
211+
visited.set(value, result)
178212

179-
// Use defineOwnProperty so special keys like __proto__ don't re-parent the result.
213+
// Use setOwn so special keys like __proto__ don't re-parent the result.
180214
for (const key in value) {
181-
defineOwnProperty(result, key, clone(value[key]))
215+
setOwn(result, key, cloneWithVisited(value[key], visited))
182216
}
183217

184218
for (const sym of Object.getOwnPropertySymbols(value)) {
185-
defineOwnProperty(result, sym, clone(value[sym]))
219+
setOwn(result, sym, cloneWithVisited(value[sym], visited))
186220
}
187221

188-
return result as any
222+
return result
189223
}
190224

191225
return value

0 commit comments

Comments
 (0)