From 6094d9491730b865d595968f8b83a1dde841f4c6 Mon Sep 17 00:00:00 2001 From: Dinh Le Date: Sun, 27 Sep 2026 10:40:02 +0700 Subject: [PATCH 1/2] fix(downgrader): convert shared schemas once per call Dereferenced documents no longer blow up: a record reached along several paths is converted once per call and its copies are shared, so a depth-22 schema diamond converts in 1 ms instead of 10.7 s and 3.2 to 3.1 on a dereferenced document drops from 8.7 s to 61 ms. --- packages/downgrader/README.md | 2 +- packages/downgrader/src/shared.test.ts | 56 ++++++++++++++++++-- packages/downgrader/src/shared.ts | 41 ++++++++++---- packages/downgrader/src/v3.1-to-v3.0.test.ts | 28 ++++++++++ packages/downgrader/src/v3.2-to-v3.1.test.ts | 12 +++++ 5 files changed, 123 insertions(+), 16 deletions(-) diff --git a/packages/downgrader/README.md b/packages/downgrader/README.md index 9869d34..e937d37 100644 --- a/packages/downgrader/README.md +++ b/packages/downgrader/README.md @@ -23,7 +23,7 @@ Every converter follows the same contract: - **Never throws.** Malformed parts are deep-copied through unchanged instead of failing the whole conversion. Cyclic object graphs, such as the output of a `$ref` dereferencer, convert with their cycles preserved. Only pathologically deep nesting (thousands of levels) can still exhaust the call stack. -- **Never mutates.** The input is left untouched and the result is a new object. +- **Never mutates.** The input is left untouched and the result is a new object. Objects shared within the input, such as a dereferenced schema used in several places, may stay shared within the result. - **Preserves extensions, never invents them.** `x-` keys and unknown keys survive. Constructs the target version cannot express are converted where an equivalent exists and removed otherwise. ## Usage diff --git a/packages/downgrader/src/shared.test.ts b/packages/downgrader/src/shared.test.ts index ba3d0fb..8911417 100644 --- a/packages/downgrader/src/shared.test.ts +++ b/packages/downgrader/src/shared.test.ts @@ -1,3 +1,5 @@ +import type { FieldTable } from './shared' + import { dig } from '../tests/helpers' import { convertRecord, @@ -236,16 +238,46 @@ describe('convertRecord', () => { expect(second.self).toBe(second) }) - it('converts shared acyclic references at every occurrence', () => { + it('converts a shared reference once per call and reuses the result', () => { const shared = { name: 'x' } + const fields: FieldTable = { name: () => 'converted' } + const convert = (item: unknown) => convertRecord(item, fields) + const result = convertRecord({ a: shared, b: shared }, { a: convert, b: convert }) + expect(result).toEqual({ a: { name: 'converted' }, b: { name: 'converted' } }) + expect(dig(result, 'b')).toBe(dig(result, 'a')) + }) + + it('clones a shared reference once per call', () => { + const shared = { deep: true } + const result = convertRecord({ a: shared, b: [shared] }, {}) + expect(result).toEqual({ a: { deep: true }, b: [{ deep: true }] }) + expect(dig(result, 'b', '0')).toBe(dig(result, 'a')) + expect(dig(result, 'a')).not.toBe(shared) + }) + + it('converts a shared reference separately for each field table and finish', () => { + const shared = { name: 'x' } + const fields: FieldTable = { name: () => 'a' } const result = convertRecord( - { a: shared, b: shared }, + { a: shared, b: shared, c: shared }, { - a: item => convertRecord(item, { name: () => 'a' }), + a: item => convertRecord(item, fields), b: item => convertRecord(item, { name: () => 'b' }), + c: item => convertRecord(item, fields, out => ({ ...out, finished: true })), }, ) - expect(result).toEqual({ a: { name: 'a' }, b: { name: 'b' } }) + expect(result).toEqual({ + a: { name: 'a' }, + b: { name: 'b' }, + c: { finished: true, name: 'a' }, + }) + }) + + it('returns fresh results on every call', () => { + const shared = { name: 'x' } + const fields: FieldTable = { name: () => 'converted' } + expect(convertRecord(shared, fields)).not.toBe(convertRecord(shared, fields)) + expect(dig(convertRecord({ a: shared }, {}), 'a')).not.toBe(dig(convertRecord({ a: shared }, {}), 'a')) }) it('releases the cycle guard when a converter throws', () => { @@ -259,6 +291,22 @@ describe('convertRecord', () => { ).toThrow('boom') expect(convertRecord(value, { a: () => 2 })).toEqual({ a: 2 }) }) + + it('forgets reused results when a converter throws', () => { + const shared = { name: 'x' } + const convert = vi.fn(() => 'converted') + const fields: FieldTable = { name: convert } + expect(() => + convertRecord({ a: shared }, { + a: (item) => { + convertRecord(item, fields) + throw new Error('boom') + }, + }), + ).toThrow('boom') + convertRecord(shared, fields) + expect(convert).toHaveBeenCalledTimes(2) + }) }) describe('operationFields', () => { diff --git a/packages/downgrader/src/shared.ts b/packages/downgrader/src/shared.ts index 721d285..3d1e282 100644 --- a/packages/downgrader/src/shared.ts +++ b/packages/downgrader/src/shared.ts @@ -41,7 +41,20 @@ export function setOwn(object: object, key: PropertyKey, value: unknown): void { } } -function cloneValue(value: unknown, seen: WeakMap): unknown { +type Finish = (out: Record, source: Record) => unknown + +interface Conversion { + done: boolean + fields: FieldTable + finish: Finish | undefined + result: unknown +} + +const conversions = new Map() +const clones = new Map() +let depth = 0 + +function cloneValue(value: unknown, seen: Map): unknown { if (!(Array.isArray(value) || isRecord(value))) { return value } @@ -69,21 +82,21 @@ export function deepClone(value: T): T { if (!(Array.isArray(value) || isRecord(value))) { return value } - return cloneValue(value, new WeakMap()) as T + return cloneValue(value, depth > 0 ? clones : new Map()) as T } -const converting = new WeakMap>() - -export function convertRecord(value: unknown, fields: FieldTable, finish?: (out: Record, source: Record) => unknown): unknown { +export function convertRecord(value: unknown, fields: FieldTable, finish?: Finish): unknown { if (!isRecord(value)) { return deepClone(value) } - const inProgress = converting.get(value) - if (inProgress !== undefined) { - return inProgress + const known = conversions.get(value) + if (known !== undefined && (!known.done || (known.fields === fields && known.finish === finish))) { + return known.result } const out: Record = {} - converting.set(value, out) + const conversion: Conversion = { done: false, fields, finish, result: out } + conversions.set(value, conversion) + depth += 1 try { for (const [key, item] of Object.entries(value)) { const convert = Object.hasOwn(fields, key) ? fields[key] : undefined @@ -95,10 +108,16 @@ export function convertRecord(value: unknown, fields: FieldTable, finish?: (out: setOwn(out, key, converted) } } - return finish === undefined ? out : finish(out, value) + conversion.result = finish === undefined ? out : finish(out, value) + conversion.done = true + return conversion.result } finally { - converting.delete(value) + depth -= 1 + if (depth === 0) { + conversions.clear() + clones.clear() + } } } diff --git a/packages/downgrader/src/v3.1-to-v3.0.test.ts b/packages/downgrader/src/v3.1-to-v3.0.test.ts index b7f7462..cfdbeb4 100644 --- a/packages/downgrader/src/v3.1-to-v3.0.test.ts +++ b/packages/downgrader/src/v3.1-to-v3.0.test.ts @@ -766,6 +766,20 @@ describe('downgradeSpecV31ToV30', () => { expect(dig(result, 'get', 'responses')).toEqual({ default: { description: '' } }) expect(dig(result, 'get', 'callbacks', 'cb', 'expr')).toBe(result) }) + + it('converts a dereferenced schema shared across the document once', () => { + const pet = { properties: { name: { type: ['string', 'null'] } }, type: 'object' } + const result = convertSpec({ + components: { schemas: { Pet: pet } }, + paths: { '/pets': { get: { responses: { 200: { content: { 'application/json': { schema: pet } }, description: 'ok' } } } } }, + }) + const schema = dig(result, 'components', 'schemas', 'Pet') + expect(schema).toEqual({ + properties: { name: { nullable: true, type: 'string' } }, + type: 'object', + }) + expect(dig(result, 'paths', '/pets', 'get', 'responses', '200', 'content', 'application/json', 'schema')).toBe(schema) + }) }) }) @@ -1363,5 +1377,19 @@ describe('downgradeSchemaV31ToV30', () => { expect(dig(result, 'properties', 'children', 'items')).toBe(result) expect(node.type).toEqual(['object', 'null']) }) + + it('converts a dereferenced schema reached along many paths once', () => { + let node: OpenAPIV3_1.SchemaObject = { type: ['string', 'null'] } + for (let index = 0; index < 64; index += 1) { + node = { properties: { left: node, right: node }, type: 'object' } + } + const result = convertSchema(node) + expect(dig(result, 'properties', 'left')).toBe(dig(result, 'properties', 'right')) + let leaf = result + for (let index = 0; index < 64; index += 1) { + leaf = dig(leaf, 'properties', 'left') + } + expect(leaf).toEqual({ nullable: true, type: 'string' }) + }) }) }) diff --git a/packages/downgrader/src/v3.2-to-v3.1.test.ts b/packages/downgrader/src/v3.2-to-v3.1.test.ts index ec73a91..06b5951 100644 --- a/packages/downgrader/src/v3.2-to-v3.1.test.ts +++ b/packages/downgrader/src/v3.2-to-v3.1.test.ts @@ -1256,6 +1256,18 @@ describe('downgradeSpecV32ToV31', () => { expect(dig(result, 'get', 'responses', '200')).toEqual({ description: 'ok' }) expect(dig(result, 'get', 'callbacks', 'cb', 'expr')).toBe(result) }) + + it('copies a dereferenced schema shared across the document once', () => { + const pet = { properties: { name: { type: 'string' } }, type: 'object' } + const result = convertSpec({ + components: { schemas: { Pet: pet } }, + paths: { '/pets': { get: { responses: { 200: { content: { 'application/json': { schema: pet } }, description: 'ok' } } } } }, + }) + const schema = dig(result, 'components', 'schemas', 'Pet') + expect(schema).toEqual(pet) + expect(schema).not.toBe(pet) + expect(dig(result, 'paths', '/pets', 'get', 'responses', '200', 'content', 'application/json', 'schema')).toBe(schema) + }) }) }) From 346f7f7d31ade9be2697c783fc91d744866a7711 Mon Sep 17 00:00:00 2001 From: Dinh Le Date: Sun, 27 Sep 2026 11:04:52 +0700 Subject: [PATCH 2/2] refactor(downgrader): derive the outermost call and close cache test gaps Drop the depth counter in favour of the conversions map size, and add tests that fail when reuse ignores the finish function, when deepClone shares copies outside a conversion, or when either cache survives a throw. --- packages/downgrader/src/shared.test.ts | 34 +++++++++++++------- packages/downgrader/src/shared.ts | 8 ++--- packages/downgrader/src/v3.2-to-v3.1.test.ts | 5 +++ 3 files changed, 31 insertions(+), 16 deletions(-) diff --git a/packages/downgrader/src/shared.test.ts b/packages/downgrader/src/shared.test.ts index 8911417..8e26212 100644 --- a/packages/downgrader/src/shared.test.ts +++ b/packages/downgrader/src/shared.test.ts @@ -135,6 +135,11 @@ describe('deepClone', () => { expect(clone.x).not.toBe(shared) expect(clone.x).toBe(clone.y) }) + + it('returns a fresh copy on every call', () => { + const shared = { a: 1 } + expect(deepClone(shared)).not.toBe(deepClone(shared)) + }) }) describe('convertRecord', () => { @@ -255,22 +260,26 @@ describe('convertRecord', () => { expect(dig(result, 'a')).not.toBe(shared) }) - it('converts a shared reference separately for each field table and finish', () => { + it('reuses a finished result only for the same field table and finish', () => { const shared = { name: 'x' } - const fields: FieldTable = { name: () => 'a' } + const fields: FieldTable = { name: () => 'converted' } + const wrap = (out: Record) => ({ wrapped: out }) const result = convertRecord( - { a: shared, b: shared, c: shared }, + { a: shared, b: shared, c: shared, d: shared }, { - a: item => convertRecord(item, fields), - b: item => convertRecord(item, { name: () => 'b' }), - c: item => convertRecord(item, fields, out => ({ ...out, finished: true })), + a: item => convertRecord(item, fields, wrap), + b: item => convertRecord(item, fields, wrap), + c: item => convertRecord(item, fields), + d: item => convertRecord(item, {}), }, ) expect(result).toEqual({ - a: { name: 'a' }, - b: { name: 'b' }, - c: { finished: true, name: 'a' }, + a: { wrapped: { name: 'converted' } }, + b: { wrapped: { name: 'converted' } }, + c: { name: 'converted' }, + d: { name: 'x' }, }) + expect(dig(result, 'b')).toBe(dig(result, 'a')) }) it('returns fresh results on every call', () => { @@ -292,20 +301,23 @@ describe('convertRecord', () => { expect(convertRecord(value, { a: () => 2 })).toEqual({ a: 2 }) }) - it('forgets reused results when a converter throws', () => { + it('forgets reused results and clones when a converter throws', () => { const shared = { name: 'x' } const convert = vi.fn(() => 'converted') const fields: FieldTable = { name: convert } + let clone: unknown expect(() => convertRecord({ a: shared }, { a: (item) => { convertRecord(item, fields) + clone = deepClone(item) throw new Error('boom') }, }), ).toThrow('boom') - convertRecord(shared, fields) + const result = convertRecord({ a: shared, b: shared }, { a: item => convertRecord(item, fields) }) expect(convert).toHaveBeenCalledTimes(2) + expect(dig(result, 'b')).not.toBe(clone) }) }) diff --git a/packages/downgrader/src/shared.ts b/packages/downgrader/src/shared.ts index 3d1e282..7909692 100644 --- a/packages/downgrader/src/shared.ts +++ b/packages/downgrader/src/shared.ts @@ -52,7 +52,6 @@ interface Conversion { const conversions = new Map() const clones = new Map() -let depth = 0 function cloneValue(value: unknown, seen: Map): unknown { if (!(Array.isArray(value) || isRecord(value))) { @@ -82,7 +81,7 @@ export function deepClone(value: T): T { if (!(Array.isArray(value) || isRecord(value))) { return value } - return cloneValue(value, depth > 0 ? clones : new Map()) as T + return cloneValue(value, conversions.size > 0 ? clones : new Map()) as T } export function convertRecord(value: unknown, fields: FieldTable, finish?: Finish): unknown { @@ -95,8 +94,8 @@ export function convertRecord(value: unknown, fields: FieldTable, finish?: Finis } const out: Record = {} const conversion: Conversion = { done: false, fields, finish, result: out } + const outermost = conversions.size === 0 conversions.set(value, conversion) - depth += 1 try { for (const [key, item] of Object.entries(value)) { const convert = Object.hasOwn(fields, key) ? fields[key] : undefined @@ -113,8 +112,7 @@ export function convertRecord(value: unknown, fields: FieldTable, finish?: Finis return conversion.result } finally { - depth -= 1 - if (depth === 0) { + if (outermost) { conversions.clear() clones.clear() } diff --git a/packages/downgrader/src/v3.2-to-v3.1.test.ts b/packages/downgrader/src/v3.2-to-v3.1.test.ts index a0314de..27eeee9 100644 --- a/packages/downgrader/src/v3.2-to-v3.1.test.ts +++ b/packages/downgrader/src/v3.2-to-v3.1.test.ts @@ -1330,6 +1330,11 @@ describe('downgradeSchemaV32ToV31', () => { }) }) + it('returns a fresh copy on every call', () => { + const schema: OpenAPIV3_2.SchemaObject = { properties: { a: { type: 'string' } }, type: 'object' } + expect(downgradeSchemaV32ToV31(schema)).not.toBe(downgradeSchemaV32ToV31(schema)) + }) + it('never mutates the input schema', () => { const schema: OpenAPIV3_2.SchemaObject = { discriminator: { defaultMapping: 'Dog', propertyName: 'kind' },