From fb81e61f7b3c173af1a7a236d6d2e4565e7dfdfa Mon Sep 17 00:00:00 2001 From: Karol Broda Date: Thu, 11 Jun 2026 09:14:00 +0200 Subject: [PATCH 1/5] feat(registry): add graph visualization (text/mermaid/dot/json) --- packages/doba/src/index.ts | 16 ++ packages/doba/src/visualize-entry.ts | 43 ++++ packages/doba/src/visualize/escape.ts | 3 + packages/doba/src/visualize/options.ts | 72 ++++++ packages/doba/src/visualize/renderers/dot.ts | 29 +++ .../doba/src/visualize/renderers/mermaid.ts | 96 ++++++++ packages/doba/src/visualize/renderers/text.ts | 70 ++++++ packages/doba/src/visualize/snapshot.ts | 34 +++ packages/doba/src/visualize/types.ts | 60 +++++ playground/src/testing.ts | 212 ++++++++++++++++++ playground/src/with-zod.ts | 5 +- 11 files changed, 639 insertions(+), 1 deletion(-) create mode 100644 packages/doba/src/visualize-entry.ts create mode 100644 packages/doba/src/visualize/escape.ts create mode 100644 packages/doba/src/visualize/options.ts create mode 100644 packages/doba/src/visualize/renderers/dot.ts create mode 100644 packages/doba/src/visualize/renderers/mermaid.ts create mode 100644 packages/doba/src/visualize/renderers/text.ts create mode 100644 packages/doba/src/visualize/snapshot.ts create mode 100644 packages/doba/src/visualize/types.ts create mode 100644 playground/src/testing.ts diff --git a/packages/doba/src/index.ts b/packages/doba/src/index.ts index ffe2e9d..2a13a56 100644 --- a/packages/doba/src/index.ts +++ b/packages/doba/src/index.ts @@ -17,6 +17,8 @@ export type { MigrationsFor, } from './migration.js' +export { MigrationConflictError } from './migration.js' + export type { NarrowExclude, PathStrategy, @@ -45,6 +47,20 @@ export type { IdentifyTransformResult, } from './registry.js' +export type { + MermaidConfig, + MermaidConfigValue, + VisualizeCommonOptions, + VisualizeDirection, + VisualizeDotOptions, + VisualizeFormat, + VisualizeInput, + VisualizeJsonOptions, + VisualizeMermaidOptions, + VisualizeOptions, + VisualizeTextOptions, +} from './visualize-entry.js' + export { pipe } from './helpers.js' export type { PipeBuilder } from './helpers.js' diff --git a/packages/doba/src/visualize-entry.ts b/packages/doba/src/visualize-entry.ts new file mode 100644 index 0000000..89845eb --- /dev/null +++ b/packages/doba/src/visualize-entry.ts @@ -0,0 +1,43 @@ +import type { Graph } from './graph.js' +import { normalizeVisualizeOptions } from './visualize/options.js' +import { renderDot } from './visualize/renderers/dot.js' +import { renderMermaid } from './visualize/renderers/mermaid.js' +import { renderText } from './visualize/renderers/text.js' +import { snapshotGraph } from './visualize/snapshot.js' +import type { VisualizeInput } from './visualize/types.js' + +export type { + MermaidConfig, + MermaidConfigValue, + VisualizeCommonOptions, + VisualizeDirection, + VisualizeDotOptions, + VisualizeFormat, + VisualizeInput, + VisualizeJsonOptions, + VisualizeMermaidOptions, + VisualizeOptions, + VisualizeTextOptions, +} from './visualize/types.js' + +export function visualizeRegistry( + graph: Graph, + input?: VisualizeInput, +): string { + const options = normalizeVisualizeOptions(input) + const snapshot = snapshotGraph(graph) + + if (options.format === 'json') { + return JSON.stringify(snapshot, null, options.space) + } + + if (options.format === 'mermaid') { + return renderMermaid(snapshot, options) + } + + if (options.format === 'dot') { + return renderDot(snapshot, options) + } + + return renderText(snapshot, options) +} diff --git a/packages/doba/src/visualize/escape.ts b/packages/doba/src/visualize/escape.ts new file mode 100644 index 0000000..06bdf9d --- /dev/null +++ b/packages/doba/src/visualize/escape.ts @@ -0,0 +1,3 @@ +export function escapeQuoted(value: string): string { + return value.replaceAll('\\', '\\\\').replaceAll('"', '\\"').replaceAll('\n', '\\n') +} diff --git a/packages/doba/src/visualize/options.ts b/packages/doba/src/visualize/options.ts new file mode 100644 index 0000000..05ee026 --- /dev/null +++ b/packages/doba/src/visualize/options.ts @@ -0,0 +1,72 @@ +import type { + NormalizedVisualizeOptions, + VisualizeDotOptions, + VisualizeInput, + VisualizeJsonOptions, + VisualizeMermaidOptions, + VisualizeTextOptions, +} from './types.js' + +export function normalizeVisualizeOptions(input: VisualizeInput = {}): NormalizedVisualizeOptions { + const format = typeof input === 'string' ? input : (input.format ?? 'text') + + if (format === 'json') { + const options: Partial = + typeof input === 'object' && input.format === 'json' ? input : {} + + return { + format, + direction: 'LR', + costs: true, + title: 'Registry graph', + graphName: 'Registry', + space: options.space ?? 2, + mermaidConfig: undefined, + } + } + + if (format === 'dot') { + const options: Partial = + typeof input === 'object' && input.format === 'dot' ? input : {} + + return { + format, + direction: options.direction ?? 'LR', + costs: options.costs ?? true, + title: 'Registry graph', + graphName: options.graphName ?? 'Registry', + space: 2, + mermaidConfig: undefined, + } + } + + if (format === 'mermaid') { + const options: Partial = + typeof input === 'object' && input.format === 'mermaid' ? input : {} + + return { + format, + direction: options.direction ?? 'LR', + costs: options.costs ?? true, + title: 'Registry graph', + graphName: 'Registry', + space: 2, + mermaidConfig: options.config, + } + } + + const options: Partial = + typeof input === 'object' && (input.format === undefined || input.format === 'text') + ? input + : {} + + return { + format, + direction: 'LR', + costs: options.costs ?? true, + title: options.title ?? 'Registry graph', + graphName: 'Registry', + space: 2, + mermaidConfig: undefined, + } +} diff --git a/packages/doba/src/visualize/renderers/dot.ts b/packages/doba/src/visualize/renderers/dot.ts new file mode 100644 index 0000000..e0ca3f8 --- /dev/null +++ b/packages/doba/src/visualize/renderers/dot.ts @@ -0,0 +1,29 @@ +import { escapeQuoted } from '../escape.js' +import type { GraphSnapshot, NormalizedVisualizeOptions } from '../types.js' + +function escapeDotIdentifier(value: string): string { + return /^[A-Za-z_][A-Za-z0-9_]*$/.test(value) ? value : `"${escapeQuoted(value)}"` +} + +export function renderDot( + snapshot: GraphSnapshot, + options: NormalizedVisualizeOptions, +): string { + const lines = [ + `digraph ${escapeDotIdentifier(options.graphName)} {`, + ` rankdir=${options.direction};`, + ] + + for (const node of snapshot.nodes) { + lines.push(` "${escapeQuoted(node)}";`) + } + + for (const edge of snapshot.edges) { + const attrs = options.costs ? ` [label="cost: ${edge.cost}"]` : '' + lines.push(` "${escapeQuoted(edge.from)}" -> "${escapeQuoted(edge.to)}"${attrs};`) + } + + lines.push('}') + + return lines.join('\n') +} diff --git a/packages/doba/src/visualize/renderers/mermaid.ts b/packages/doba/src/visualize/renderers/mermaid.ts new file mode 100644 index 0000000..b665a8b --- /dev/null +++ b/packages/doba/src/visualize/renderers/mermaid.ts @@ -0,0 +1,96 @@ +import { escapeQuoted } from '../escape.js' +import type { GraphSnapshot, MermaidConfig, NormalizedVisualizeOptions } from '../types.js' + +function formatYamlScalar(value: unknown): string { + if (value === null || value === undefined) { + return 'null' + } + + if (typeof value === 'string') { + return /^[A-Za-z0-9_-]+$/.test(value) ? value : JSON.stringify(value) + } + + if (typeof value === 'number' || typeof value === 'boolean') { + return String(value) + } + + return JSON.stringify(String(value)) +} + +function formatYamlValue(value: unknown, indent: number): string[] { + if (Array.isArray(value)) { + return value.flatMap((item) => { + if (typeof item === 'object' && item !== null && !Array.isArray(item)) { + return [' '.repeat(indent) + '-', ...formatYamlObject(item, indent + 2)] + } + + return [' '.repeat(indent) + `- ${formatYamlScalar(item)}`] + }) + } + + if (typeof value === 'object' && value !== null) { + return formatYamlObject(value, indent) + } + + return [' '.repeat(indent) + formatYamlScalar(value)] +} + +function formatYamlObject(config: object, indent: number): string[] { + const lines: string[] = [] + + for (const [key, value] of Object.entries(config)) { + const prefix = ' '.repeat(indent) + `${key}:` + + if (typeof value === 'object' && value !== null) { + lines.push(prefix, ...formatYamlValue(value, indent + 2)) + continue + } + + lines.push(`${prefix} ${formatYamlScalar(value)}`) + } + + return lines +} + +function renderMermaidFrontmatter(config: MermaidConfig | undefined): string[] { + if (config === undefined) { + return [] + } + + return ['---', 'config:', ...formatYamlObject(config, 2), '---'] +} + +export function renderMermaid( + snapshot: GraphSnapshot, + options: NormalizedVisualizeOptions, +): string { + const nodeIds = new Map() + const lines = [ + ...renderMermaidFrontmatter(options.mermaidConfig), + `flowchart ${options.direction}`, + ] + + for (let index = 0; index < snapshot.nodes.length; index++) { + const node = snapshot.nodes[index] + if (node === undefined) { + continue + } + + const nodeId = `n${index}` + nodeIds.set(node, nodeId) + lines.push(` ${nodeId}["${escapeQuoted(node)}"]`) + } + + for (const edge of snapshot.edges) { + const from = nodeIds.get(edge.from) + const to = nodeIds.get(edge.to) + if (from === undefined || to === undefined) { + continue + } + + const label = options.costs ? `|cost: ${edge.cost}|` : '' + lines.push(` ${from} -->${label} ${to}`) + } + + return lines.join('\n') +} diff --git a/packages/doba/src/visualize/renderers/text.ts b/packages/doba/src/visualize/renderers/text.ts new file mode 100644 index 0000000..f58e569 --- /dev/null +++ b/packages/doba/src/visualize/renderers/text.ts @@ -0,0 +1,70 @@ +import type { GraphEdge, GraphSnapshot, NormalizedVisualizeOptions } from '../types.js' + +function pluralize(count: number, singular: string, plural: string): string { + return count === 1 ? singular : plural +} + +function formatMigration( + migration: GraphEdge, + options: NormalizedVisualizeOptions, +): string { + if (options.costs) { + return `${migration.to} (cost: ${migration.cost})` + } + + return migration.to +} + +function migrationsBySource(snapshot: GraphSnapshot): Map[]> { + const migrations = new Map[]>() + + for (const node of snapshot.nodes) { + migrations.set(node, []) + } + + for (const edge of snapshot.edges) { + const outgoing = migrations.get(edge.from) + if (outgoing !== undefined) { + outgoing.push(edge) + } + } + + return migrations +} + +export function renderText( + snapshot: GraphSnapshot, + options: NormalizedVisualizeOptions, +): string { + const schemaLabel = pluralize(snapshot.nodes.length, 'schema', 'schemas') + const migrationLabel = pluralize(snapshot.edges.length, 'migration', 'migrations') + const lines = [ + `${options.title} (${snapshot.nodes.length} ${schemaLabel}, ${snapshot.edges.length} ${migrationLabel})`, + '', + ] + const outgoingMigrations = migrationsBySource(snapshot) + + for (const node of snapshot.nodes) { + const migrations = outgoingMigrations.get(node) + lines.push(`● ${node}`) + + if (migrations === undefined || migrations.length === 0) { + lines.push(' └─ no outgoing migrations', '') + continue + } + + for (let index = 0; index < migrations.length; index++) { + const migration = migrations[index] + if (migration === undefined) { + continue + } + + const branch = index === migrations.length - 1 ? '└' : '├' + lines.push(` ${branch}─▶ ${formatMigration(migration, options)}`) + } + + lines.push('') + } + + return lines.join('\n').trimEnd() +} diff --git a/packages/doba/src/visualize/snapshot.ts b/packages/doba/src/visualize/snapshot.ts new file mode 100644 index 0000000..5b9808a --- /dev/null +++ b/packages/doba/src/visualize/snapshot.ts @@ -0,0 +1,34 @@ +import type { Graph } from '../graph.js' +import type { GraphEdge, GraphSnapshot } from './types.js' + +function compareEdges(left: GraphEdge, right: GraphEdge): number { + const from = left.from.localeCompare(right.from) + if (from !== 0) { + return from + } + + const to = left.to.localeCompare(right.to) + if (to !== 0) { + return to + } + + return left.cost - right.cost +} + +export function snapshotGraph(graph: Graph): GraphSnapshot { + const nodes = new Set() + const edges: GraphEdge[] = [] + + for (const [from, outgoing] of graph) { + nodes.add(from) + for (const edge of outgoing) { + nodes.add(edge.to) + edges.push({ from, to: edge.to, cost: edge.cost }) + } + } + + return { + nodes: [...nodes].toSorted(), + edges: edges.toSorted(compareEdges), + } +} diff --git a/packages/doba/src/visualize/types.ts b/packages/doba/src/visualize/types.ts new file mode 100644 index 0000000..2634f82 --- /dev/null +++ b/packages/doba/src/visualize/types.ts @@ -0,0 +1,60 @@ +export type VisualizeFormat = 'text' | 'mermaid' | 'json' | 'dot' +export type VisualizeDirection = 'LR' | 'TB' + +export type MermaidConfigValue = unknown +export type MermaidConfig = Readonly> + +export type VisualizeCommonOptions = { + readonly costs?: boolean | undefined +} + +export type VisualizeTextOptions = VisualizeCommonOptions & { + readonly format?: 'text' | undefined + readonly title?: string | undefined +} + +export type VisualizeMermaidOptions = VisualizeCommonOptions & { + readonly format: 'mermaid' + readonly direction?: VisualizeDirection | undefined + readonly config?: MermaidConfig | undefined +} + +export type VisualizeDotOptions = VisualizeCommonOptions & { + readonly format: 'dot' + readonly direction?: VisualizeDirection | undefined + readonly graphName?: string | undefined +} + +export type VisualizeJsonOptions = { + readonly format: 'json' + readonly space?: number | undefined +} + +export type VisualizeOptions = + | VisualizeTextOptions + | VisualizeMermaidOptions + | VisualizeDotOptions + | VisualizeJsonOptions + +export type VisualizeInput = VisualizeFormat | VisualizeOptions + +export type NormalizedVisualizeOptions = { + readonly format: VisualizeFormat + readonly direction: VisualizeDirection + readonly costs: boolean + readonly title: string + readonly graphName: string + readonly space: number + readonly mermaidConfig: MermaidConfig | undefined +} + +export type GraphEdge = { + readonly from: K + readonly to: K + readonly cost: number +} + +export type GraphSnapshot = { + readonly nodes: readonly K[] + readonly edges: readonly GraphEdge[] +} diff --git a/playground/src/testing.ts b/playground/src/testing.ts new file mode 100644 index 0000000..d755cb0 --- /dev/null +++ b/playground/src/testing.ts @@ -0,0 +1,212 @@ +/* oxlint-disable no-console -- playground demo script */ +/* oxlint-disable prefer-top-level-await -- wrapped in main() for error handling */ +import { createRegistry } from 'dobajs' +import { z } from 'zod' + +const legacySchema = z.object({ + legacyId: z.number(), + fullName: z.string(), + emailAddress: z.string().optional(), + isAdmin: z.boolean(), +}) + +const accountSchema = z.object({ + id: z.string(), + name: z.string(), + email: z.string(), + role: z.enum(['admin', 'member']), +}) + +const profileSchema = z.object({ + id: z.string(), + displayName: z.string(), + email: z.string(), + role: z.enum(['admin', 'member']), + verified: z.boolean(), +}) + +const publicProfileSchema = z.object({ + id: z.string(), + displayName: z.string(), +}) + +const analyticsUserSchema = z.object({ + userId: z.string(), + segment: z.enum(['staff', 'customer']), + contactable: z.boolean(), +}) + +const archiveSchema = z.object({ + key: z.string(), + payload: z.string(), +}) + +const auditSchema = z.object({ + id: z.string(), + actor: z.string(), + action: z.string(), +}) + +const registry = createRegistry({ + schemas: { + legacy: legacySchema, + account: accountSchema, + profile: profileSchema, + publicProfile: publicProfileSchema, + analyticsUser: analyticsUserSchema, + archive: archiveSchema, + audit: auditSchema, + }, + + migrations: { + 'legacy->account': { + migrate: (user, context) => { + const email = + user.emailAddress ?? `${user.fullName.toLowerCase().replaceAll(/\s+/g, '.')}@example.com` + + if (user.emailAddress === undefined) { + context.defaulted(['email'], 'generated from fullName') + } + + return { + id: `legacy-${user.legacyId}`, + name: user.fullName, + email, + role: user.isAdmin ? 'admin' : 'member', + } + }, + preferred: true, + label: 'normalize-legacy-user', + }, + + 'account->profile': { + migrate: (user, context) => { + context.defaulted(['verified'], 'new profiles start unverified') + + return { + id: user.id, + displayName: user.name, + email: user.email, + role: user.role, + verified: false, + } + }, + preferred: true, + label: 'account-to-profile', + }, + + 'legacy->profile': { + migrate: (user, context) => { + context.warn('legacy->profile skips account normalization') + + return { + id: `legacy-${user.legacyId}`, + displayName: user.fullName, + email: user.emailAddress ?? 'unknown@example.com', + role: user.isAdmin ? 'admin' : 'member', + verified: false, + } + }, + deprecated: 'prefer legacy -> account -> profile', + label: 'legacy-direct-profile', + }, + + 'profile->publicProfile': { + pipe: (builder) => builder.drop('email').drop('role').drop('verified'), + label: 'strip-private-profile-fields', + }, + + 'account->analyticsUser': { + migrate: (user) => ({ + userId: user.id, + segment: user.role === 'admin' ? 'staff' : 'customer', + contactable: user.email.length > 0, + }), + cost: 2, + label: 'account-to-analytics', + }, + + 'profile->analyticsUser': { + migrate: (user) => ({ + userId: user.id, + segment: user.role === 'admin' ? 'staff' : 'customer', + contactable: user.verified, + }), + cost: 5, + label: 'profile-to-analytics', + }, + + 'analyticsUser->archive': { + migrate: (user) => ({ + key: user.userId, + payload: JSON.stringify(user), + }), + label: 'archive-analytics-user', + }, + + 'publicProfile->archive': { + migrate: (profile) => ({ + key: profile.id, + payload: JSON.stringify(profile), + }), + cost: 4, + label: 'archive-public-profile', + }, + + 'account<->audit': { + forward: (user) => ({ + id: user.id, + actor: user.name, + action: `account:${user.role}`, + }), + backward: (audit) => ({ + id: audit.id, + name: audit.actor, + email: `${audit.actor.toLowerCase().replaceAll(/\s+/g, '.')}@example.com`, + role: audit.action.includes('admin') ? 'admin' : 'member', + }), + cost: 3, + label: 'account-audit-roundtrip', + }, + }, +}) + +async function main() { + const legacyUser = { + legacyId: 42, + fullName: 'Alice Smith', + isAdmin: true, + } + + console.log(registry.visualize()) + console.log('\n--- mermaid ---') + console.log( + registry.visualize({ + format: 'mermaid', + direction: 'TB', + config: { + theme: 'neutral', + layout: 'elk', + }, + }), + ) + console.log('\n--- dot ---') + console.log(registry.visualize({ format: 'dot', graphName: 'PlaygroundRegistry' })) + console.log('\n--- json ---') + console.log(registry.visualize({ format: 'json', space: 2 })) + + console.log('\n--- path choices ---') + console.log('legacy -> profile', registry.findPath('legacy', 'profile')) + console.log('legacy -> analyticsUser', registry.findPath('legacy', 'analyticsUser')) + console.log('legacy -> archive', registry.findPath('legacy', 'archive')) + console.log('audit -> publicProfile', registry.findPath('audit', 'publicProfile')) + + console.log('\n--- transform ---') + const result = await registry.transform(legacyUser, 'legacy', 'archive', { + validate: 'none', + }) + + console.log(result) +} + +main().catch(console.error) diff --git a/playground/src/with-zod.ts b/playground/src/with-zod.ts index 18f78ef..1ad976d 100644 --- a/playground/src/with-zod.ts +++ b/playground/src/with-zod.ts @@ -123,7 +123,7 @@ const userRegistry = createRegistry({ }, } }, - 'frontend->frontend:v2': (user) => user, + 'frontend->frontend:v2': { pipe: (p) => p }, 'frontend:v2->frontend': (user) => user, }, @@ -241,6 +241,9 @@ async function main() { if (!invalidResult.ok) { log(invalidResult.issues, 'validate:frontend:issues') } + + const visualization = userRegistry.visualize() + console.log(visualization) } main().catch(console.error) From 6980e15f116f323b4215ef688c0b395bd5f10aa3 Mon Sep 17 00:00:00 2001 From: Karol Broda Date: Thu, 11 Jun 2026 09:42:00 +0200 Subject: [PATCH 2/5] playground: switch helpers demo to inline pipe builder form --- playground/src/with-helpers.ts | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/playground/src/with-helpers.ts b/playground/src/with-helpers.ts index 6592cb1..7acf43e 100644 --- a/playground/src/with-helpers.ts +++ b/playground/src/with-helpers.ts @@ -1,7 +1,7 @@ /* oxlint-disable no-console -- playground demo scripts use console for output */ /* oxlint-disable prefer-top-level-await -- wrapped in main() for error handling */ import { z } from 'zod' -import { createRegistry, pipe } from 'dobajs' +import { createRegistry } from 'dobajs' import { log } from './log' @@ -24,22 +24,22 @@ const v3Schema = z.object({ verified: z.boolean(), }) -type V1 = z.infer -type V2 = z.infer - const registry = createRegistry({ schemas: { v1: v1Schema, v2: v2Schema, v3: v3Schema }, migrations: { // type-safe builder: field names autocomplete, map callback is typed - 'v1->v2': pipe() - .rename('userName', 'name') - .map('isAdmin', (v) => (v ? 'admin' : 'user')) - .rename('isAdmin', 'role') - .drop('legacyId') - .add('email', () => 'unknown@example.com'), + 'v1->v2': { + pipe: (p) => + p + .rename('userName', 'name') + .map('isAdmin', (v) => (v ? 'admin' : 'user')) + .rename('isAdmin', 'role') + .drop('legacyId') + .add('email', () => 'unknown@example.com'), + }, - 'v2->v3': pipe().add('verified', false), + 'v2->v3': { pipe: (p) => p.add('verified', false) }, }, hooks: { @@ -55,7 +55,9 @@ async function main() { console.log() log('v1 -> v2 using pipe().rename().map().rename().drop().add()', 'transform') - const v2Result = await registry.transform(user, 'v1', 'v2', { validate: 'none' }) + const v2Result = await registry.transform(user, 'v1', 'v2', { + validate: 'none', + }) if (v2Result.ok) { log(v2Result.value, 'result') log(v2Result.meta.defaults, 'defaults') @@ -63,7 +65,9 @@ async function main() { console.log() log('v1 -> v3 auto-chaining through v2', 'transform') - const v3Result = await registry.transform(user, 'v1', 'v3', { validate: 'none' }) + const v3Result = await registry.transform(user, 'v1', 'v3', { + validate: 'none', + }) if (v3Result.ok) { log(v3Result.value, 'result') log(v3Result.meta.path, 'path') From b9ae27d90d07731c1c9021ae9532f99f005575f9 Mon Sep 17 00:00:00 2001 From: Karol Broda Date: Thu, 11 Jun 2026 14:08:00 +0200 Subject: [PATCH 3/5] fix(registry): harden hooks, paths, identify, and migration resolution --- .../__snapshots__/tsnapi/index.snapshot.d.ts | 35 ++ .../__snapshots__/tsnapi/index.snapshot.js | 8 + packages/doba/src/context.ts | 4 +- packages/doba/src/helpers.ts | 24 +- packages/doba/src/identify.ts | 19 +- packages/doba/src/migration.ts | 77 +++- packages/doba/src/registry.ts | 370 ++++++++++++++---- packages/doba/src/transform.ts | 70 +++- 8 files changed, 496 insertions(+), 111 deletions(-) diff --git a/packages/doba/__snapshots__/tsnapi/index.snapshot.d.ts b/packages/doba/__snapshots__/tsnapi/index.snapshot.d.ts index 74ce082..7effc83 100644 --- a/packages/doba/__snapshots__/tsnapi/index.snapshot.d.ts +++ b/packages/doba/__snapshots__/tsnapi/index.snapshot.d.ts @@ -60,6 +60,8 @@ export type IdentifyTransformMeta = TransformMeta< readonly from: Keys; }; export type IdentifyTransformResult = Result>; +export type MermaidConfig = Readonly>; +export type MermaidConfigValue = unknown; export type MigrationDef = MigrationFn | (MigrationMetadata & { readonly migrate: MigrationFn; }) | PipeMigrationDef; @@ -151,6 +153,31 @@ export type ValidateMeta = { readonly schema: Key; }; export type ValidateResult = Result>; +export type VisualizeCommonOptions = { + readonly costs?: boolean | undefined; +}; +export type VisualizeDirection = "LR" | "TB"; +export type VisualizeDotOptions = VisualizeCommonOptions & { + readonly format: "dot"; + readonly direction?: VisualizeDirection | undefined; + readonly graphName?: string | undefined; +}; +export type VisualizeFormat = "text" | "mermaid" | "json" | "dot"; +export type VisualizeInput = VisualizeFormat | VisualizeOptions; +export type VisualizeJsonOptions = { + readonly format: "json"; + readonly space?: number | undefined; +}; +export type VisualizeMermaidOptions = VisualizeCommonOptions & { + readonly format: "mermaid"; + readonly direction?: VisualizeDirection | undefined; + readonly config?: MermaidConfig | undefined; +}; +export type VisualizeOptions = VisualizeTextOptions | VisualizeMermaidOptions | VisualizeDotOptions | VisualizeJsonOptions; +export type VisualizeTextOptions = VisualizeCommonOptions & { + readonly format?: "text" | undefined; + readonly title?: string | undefined; +}; export type WarningInfo = { readonly message: string; readonly from: FromKey; @@ -158,6 +185,14 @@ export type WarningInfo string | null; export declare function createRegistry(_: RegistryConfig): Registry; diff --git a/packages/doba/__snapshots__/tsnapi/index.snapshot.js b/packages/doba/__snapshots__/tsnapi/index.snapshot.js index 44674ff..1f6d7e2 100644 --- a/packages/doba/__snapshots__/tsnapi/index.snapshot.js +++ b/packages/doba/__snapshots__/tsnapi/index.snapshot.js @@ -1,6 +1,14 @@ /** * Generated by tsnapi — public API snapshot of `dobajs` */ +// #region Classes +export class MigrationConflictError extends Error { + edge + sources + constructor(_, _) {} +} +// #endregion + // #region Functions export function byField(_, _) {} export function createRegistry(_) {} diff --git a/packages/doba/src/context.ts b/packages/doba/src/context.ts index 6e00549..9b31878 100644 --- a/packages/doba/src/context.ts +++ b/packages/doba/src/context.ts @@ -71,7 +71,9 @@ export function createTransformContext 0 ? path.join('.') : '(root)' + // Stringify each segment safely: path.join('.') throws on Symbol entries + // because String(Symbol) throws a TypeError. + const pathStr = path.length > 0 ? path.map((segment) => String(segment)).join('.') : '(root)' onWarning?.(`defaulted ${pathStr}: ${message}`, from, to) }, } diff --git a/packages/doba/src/helpers.ts b/packages/doba/src/helpers.ts index e41ff6f..1f68438 100644 --- a/packages/doba/src/helpers.ts +++ b/packages/doba/src/helpers.ts @@ -8,6 +8,26 @@ type PipeOp = | { type: 'drop'; names: string[] } | { type: 'map'; name: string; fn: (value: unknown) => unknown } +/** + * returns a fresh copy of object/array defaults so callers can't mutate one + * transform's result and affect the next. factory functions and primitives + * are returned as-is. + */ +function cloneDefaultValue(value: unknown): unknown { + if (value === null || typeof value !== 'object') { + return value + } + if (Array.isArray(value)) { + return value.map((item) => + item !== null && typeof item === 'object' ? cloneDefaultValue(item) : item, + ) + } + // plain object: shallow-clone is sufficient for top-level isolation; nested + // mutation is documented as out-of-scope (matches the shallow spread used + // elsewhere in executePipe). + return { ...(value as Record) } +} + /** flattens intersections for readable IDE tooltips. */ // eslint-disable-next-line typescript-eslint/ban-types -- intentional empty intersection for type simplification type Simplify = { [K in keyof T]: T[K] } & {} @@ -119,7 +139,9 @@ function executePipe( case 'add': if (!(op.name in result)) { result[op.name] = - typeof op.defaultValue === 'function' ? op.defaultValue() : op.defaultValue + typeof op.defaultValue === 'function' + ? op.defaultValue() + : cloneDefaultValue(op.defaultValue) ctx.defaulted([op.name], `added with default`) } break diff --git a/packages/doba/src/identify.ts b/packages/doba/src/identify.ts index de85d65..084eaec 100644 --- a/packages/doba/src/identify.ts +++ b/packages/doba/src/identify.ts @@ -86,6 +86,11 @@ type ByFieldOptions = /** * creates an identify function that reads a field from the value and maps it to a schema key. * + * non-string field values (numbers, booleans, null, objects) return `null` + * instead of being stringified -- stringifying them would produce plausible- + * looking keys like "[object Object]" or "123" that silently fail to match + * any schema. coerce explicitly before passing to byField if you need that. + * * @example * ```ts * // value.version matches schema key directly @@ -108,7 +113,10 @@ export function byField( if (!isObj(value) || !(field in value)) { return null } - const raw = String(value[field]) + const raw = value[field] + if (typeof raw !== 'string') { + return null + } return mapping[raw] ?? null } } @@ -121,7 +129,8 @@ export function byField( if (!isObj(value) || !(field in value)) { return null } - return String(value[field]) + const raw = value[field] + return typeof raw === 'string' ? raw : null } } @@ -129,7 +138,11 @@ export function byField( if (!isObj(value) || !(field in value)) { return null } - return `${prefix}${String(value[field])}${suffix}` + const raw = value[field] + if (typeof raw !== 'string') { + return null + } + return `${prefix}${raw}${suffix}` } } diff --git a/packages/doba/src/migration.ts b/packages/doba/src/migration.ts index 58f7d68..65e1925 100644 --- a/packages/doba/src/migration.ts +++ b/packages/doba/src/migration.ts @@ -163,6 +163,23 @@ export const PREFERRED_COST = 0 /** edge cost assigned when a migration is deprecated, making the graph avoid it. */ export const DEPRECATED_COST = 1000 +/** + * thrown by {@link resolveMigrations} when two definitions claim the same edge + * and neither overrides the other (e.g. two reversible migrations that both + * register `a<->b`). surfaced as a typed error so callers building registries + * from dynamic config can distinguish it from unrelated failures. + */ +export class MigrationConflictError extends Error { + readonly edge: string + readonly sources: readonly [string, string] + constructor(edge: string, sources: readonly [string, string]) { + super(`Migration conflict: "${edge}" is defined by both "${sources[0]}" and "${sources[1]}"`) + this.name = 'MigrationConflictError' + this.edge = edge + this.sources = sources + } +} + export type MigrationFunction = (value: unknown, ctx: unknown) => unknown export type ResolvedMigration = { readonly fn: MigrationFunction @@ -191,7 +208,7 @@ function extractMetadata(obj: Record): { } let resolvedCost: number = DEFAULT_COST - if (typeof cost === 'number') { + if (typeof cost === 'number' && Number.isFinite(cost) && cost >= 0) { resolvedCost = cost } else if (preferred === true) { resolvedCost = PREFERRED_COST @@ -247,9 +264,7 @@ function registerEdge( }) } else { // same kind (two one-ways or two reversibles) - throw new Error( - `Migration conflict: "${edgeKey}" is defined by both "${existing}" and "${sourceKey}"`, - ) + throw new MigrationConflictError(edgeKey, [existing, sourceKey]) } return } @@ -271,6 +286,15 @@ export function resolveMigrations(migrations: object): ResolveMigrationsResult { const sources = new Map() const warnings: MigrationWarning[] = [] + /** + * pushes a "skipped migration" warning. previously these were silent + * `continue`s, which let typos like `backwards:` instead of `backward:` + * register half a reversible migration with no signal. + */ + const skip = (key: string, from: string, to: string, reason: string): void => { + warnings.push({ from, to, message: `skipped migration "${key}": ${reason}` }) + } + for (const [key, def] of typedEntries(migrations)) { if (def === undefined) { continue @@ -278,13 +302,31 @@ export function resolveMigrations(migrations: object): ResolveMigrationsResult { const reversibleIdx = key.indexOf('<->') if (reversibleIdx !== -1) { - if (!isObject(def) || !('forward' in def) || !('backward' in def)) { + const from = key.slice(0, reversibleIdx) + const to = key.slice(reversibleIdx + 3) + + // T2: reject keys whose parsed endpoints themselves contain arrow + // tokens -- they would silently mis-parse. e.g. "a<->b<->c" parses as + // from="a", to="b<->c", neither of which is what the user intended. + if ( + from.length === 0 || + to.length === 0 || + from.includes('<->') || + to.includes('<->') || + from.includes('->') || + to.includes('->') + ) { + skip( + key, + from, + to, + 'malformed key (use exactly one "<->" between two non-empty schema names)', + ) continue } - const from = key.slice(0, reversibleIdx) - const to = key.slice(reversibleIdx + 3) - if (from.length === 0 || to.length === 0) { + if (!isObject(def) || !('forward' in def) || !('backward' in def)) { + skip(key, from, to, 'reversible migration must have "forward" and "backward" functions') continue } @@ -294,6 +336,7 @@ export function resolveMigrations(migrations: object): ResolveMigrationsResult { const { forward } = def const { backward } = def if (typeof forward !== 'function' || typeof backward !== 'function') { + skip(key, from, to, '"forward" and "backward" must be functions') continue } @@ -310,12 +353,23 @@ export function resolveMigrations(migrations: object): ResolveMigrationsResult { const arrowIdx = key.indexOf('->') if (arrowIdx === -1) { + skip(key, '', '', 'malformed key (use "from->to" or "from<->to")') continue } const from = key.slice(0, arrowIdx) const to = key.slice(arrowIdx + 2) - if (from.length === 0 || to.length === 0) { + // T2: a key like "a->b->c" parses as from="a", to="b->c" -- the trailing + // arrow in `to` is almost never intended. reject it explicitly. + if ( + from.length === 0 || + to.length === 0 || + from.includes('->') || + to.includes('->') || + from.includes('<->') || + to.includes('<->') + ) { + skip(key, from, to, 'malformed key (use exactly one "->" between two non-empty schema names)') continue } @@ -330,10 +384,12 @@ export function resolveMigrations(migrations: object): ResolveMigrationsResult { } else if (isObject(def) && 'pipe' in def) { const { pipe: pipeFn } = def if (typeof pipeFn !== 'function') { + skip(key, from, to, '"pipe" must be a function') continue } const migrationFn = pipeFn(createPipe()) as MigrationFunction if (typeof migrationFn !== 'function') { + skip(key, from, to, 'pipe callback must return a function') continue } const meta = extractMetadata(def) @@ -345,6 +401,7 @@ export function resolveMigrations(migrations: object): ResolveMigrationsResult { } else if (isObject(def) && 'migrate' in def) { const { migrate } = def if (typeof migrate !== 'function') { + skip(key, from, to, '"migrate" must be a function') continue } const meta = extractMetadata(def) @@ -353,6 +410,8 @@ export function resolveMigrations(migrations: object): ResolveMigrationsResult { fn: migrate as MigrationFunction, source: key, }) + } else { + skip(key, from, to, 'must be a function, { migrate }, { pipe }, or { forward, backward }') } } diff --git a/packages/doba/src/registry.ts b/packages/doba/src/registry.ts index 613ddb2..08d35b3 100644 --- a/packages/doba/src/registry.ts +++ b/packages/doba/src/registry.ts @@ -29,6 +29,53 @@ import { validateWithSchema, } from './transform.js' import { type IdentifyGuard, type TryParse, tryParse } from './identify.js' +import { type VisualizeInput, visualizeRegistry } from './visualize-entry.js' + +/** + * invokes `fn` and returns `undefined` if it throws. observer-pattern hook + * invocation: a throwing hook must NOT escape the registry and break the + * Result contract. swallowed errors are surfaced via `onSwallow` so callers + * (e.g. `debug` mode) can still observe them. + */ +function safeCall( + fn: (() => T) | undefined, + onSwallow?: (error: unknown) => void, +): T | undefined { + if (fn === undefined) { + return undefined + } + try { + return fn() + } catch (error) { + onSwallow?.(error) + return undefined + } +} + +/** standard swallow sink: only logs when debug mode is on. */ +function debugSwallow(debug: boolean | undefined, hookName: string): (error: unknown) => void { + if (!debug) { + return () => {} + } + return (error: unknown) => { + const message = error instanceof Error ? error.message : String(error) + // oxlint-disable-next-line no-console -- debug mode intentionally logs swallowed hook errors + console.log(`[doba] ${hookName} hook threw (swallowed): ${message}`) + } +} + +/** + * thenable detector shared by transform, validate, and identify. `instanceof + * Promise` misses values returned by libraries that produce promise-like + * objects without using the global Promise constructor. + */ +function isThenable(value: unknown): value is PromiseLike { + return ( + value !== null && + typeof value === 'object' && + typeof (value as { then?: unknown }).then === 'function' + ) +} /** information passed to the {@link RegistryHooks.onTransform} hook. */ export type TransformHookInfo = { @@ -187,14 +234,20 @@ type RegistryBase = { /** * returns a diagnostic description of the migration path between two schemas, * including costs, labels, deprecation info, and a human-readable summary. + * + * `options.pathStrategy` overrides the registry-level strategy for this call, + * matching the per-call override accepted by {@link Registry.transform}. */ readonly explain: , To extends SchemaKeys>( from: From, to: To, + options?: { readonly pathStrategy?: PathStrategy | undefined } | undefined, ) => ExplainResult, From, To> /** the schemas this registry was created with. */ readonly schemas: Schemas + + readonly visualize: (input?: VisualizeInput) => string } /** methods added to the registry when identify is configured. */ @@ -362,6 +415,12 @@ export function createRegistry( // when no timing hooks are registered, skip all performance.now() calls and object allocations const hasTiming = onTransform !== undefined || onStep !== undefined + // sinks for swallowed hook errors (H1-H5): a throwing hook never escapes the + // registry; in debug mode the swallowed error is logged. + const swallowWarning = debugSwallow(debug, 'onWarning') + const swallowStep = debugSwallow(debug, 'onStep') + const swallowTransform = debugSwallow(debug, 'onTransform') + for (const w of resolved.warnings) { onWarning?.(w.message, w.from as Keys, w.to as Keys) } @@ -440,6 +499,19 @@ export function createRegistry( const totalSteps = path.length - 1 let current = value + // T3: 'each' validates intermediate schemas; the input schema (path[0]) + // is validated here so a malformed input doesn't pass through unchecked. + if (validateStrategy === 'each' && totalSteps > 0) { + const inputSchema = schemas[path[0] as Keys] + if (inputSchema !== undefined) { + const result = await validateWithSchema(inputSchema, current, path[0] as Keys) + if (!result.ok) { + return result + } + current = result.value + } + } + for (let i = 0; i < totalSteps; i++) { const stepFrom = path[i] as Keys const stepTo = path[i + 1] as Keys @@ -467,15 +539,22 @@ export function createRegistry( from: stepFrom, to: stepTo, }) - onWarning?.(`using deprecated migration "${key}"${reason}`, stepFrom, stepTo) + // H5: deprecated-warning invocation must not escape the registry. + safeCall( + () => onWarning?.(`using deprecated migration "${key}"${reason}`, stepFrom, stepTo), + swallowWarning, + ) } - const ctx = createTransformContext(state, stepFrom, stepTo, onWarning) + const ctx = createTransformContext(state, stepFrom, stepTo, (msg, f, t) => + safeCall(() => onWarning?.(msg, f, t), swallowWarning), + ) try { const result = migration.fn(current, ctx) + // T11: detect thenables, not just real Promises. // oxlint-disable-next-line no-await-in-loop -- migration steps must run sequentially, each depends on the previous result - current = result instanceof Promise ? await result : result + current = isThenable(result) ? await result : (result as unknown) } catch (error) { const message = error instanceof Error ? error.message : String(error) return err([createIssue('transform_failed', `migration "${key}" threw: ${message}`)]) @@ -528,6 +607,30 @@ export function createRegistry( const totalSteps = path.length - 1 let current = value + // T3: 'each' validates intermediate schemas; the input schema (path[0]) + // is validated here so a malformed input doesn't pass through unchecked. + if (validateStrategy === 'each' && totalSteps > 0) { + const inputSchema = schemas[path[0] as Keys] + if (inputSchema !== undefined) { + const result = await validateWithSchema(inputSchema, current, path[0] as Keys) + if (!result.ok) { + safeCall( + () => + onTransform?.({ + from, + to, + path, + durationMs: performance.now() - startTime, + ok: false, + }), + swallowTransform, + ) + return result + } + current = result.value + } + } + for (let i = 0; i < totalSteps; i++) { const stepFrom = path[i] as Keys const stepTo = path[i + 1] as Keys @@ -536,7 +639,12 @@ export function createRegistry( if (migration === undefined) { const r = err([createIssue('transform_failed', `missing migration "${key}"`)]) - onTransform?.({ from, to, path, durationMs: performance.now() - startTime, ok: false }) + // H2: onTransform invocations must not escape the registry. + safeCall( + () => + onTransform?.({ from, to, path, durationMs: performance.now() - startTime, ok: false }), + swallowTransform, + ) return r } @@ -557,38 +665,58 @@ export function createRegistry( from: stepFrom, to: stepTo, }) - onWarning?.(`using deprecated migration "${key}"${reason}`, stepFrom, stepTo) + // H5: deprecated-warning invocation must not escape the registry. + safeCall( + () => onWarning?.(`using deprecated migration "${key}"${reason}`, stepFrom, stepTo), + swallowWarning, + ) } - const ctx = createTransformContext(state, stepFrom, stepTo, onWarning) + const ctx = createTransformContext(state, stepFrom, stepTo, (msg, f, t) => + safeCall(() => onWarning?.(msg, f, t), swallowWarning), + ) const stepStart = performance.now() try { const result = migration.fn(current, ctx) + // T11: detect thenables, not just real Promises. // oxlint-disable-next-line no-await-in-loop -- migration steps must run sequentially, each depends on the previous result - current = result instanceof Promise ? await result : result - onStep?.({ - from: stepFrom, - to: stepTo, - index: i, - total: totalSteps, - label: migration.label, - durationMs: performance.now() - stepStart, - ok: true, - }) + current = isThenable(result) ? await result : (result as unknown) + // H1: onStep invocations must not escape the registry. + safeCall( + () => + onStep?.({ + from: stepFrom, + to: stepTo, + index: i, + total: totalSteps, + label: migration.label, + durationMs: performance.now() - stepStart, + ok: true, + }), + swallowStep, + ) } catch (error) { - onStep?.({ - from: stepFrom, - to: stepTo, - index: i, - total: totalSteps, - label: migration.label, - durationMs: performance.now() - stepStart, - ok: false, - }) + safeCall( + () => + onStep?.({ + from: stepFrom, + to: stepTo, + index: i, + total: totalSteps, + label: migration.label, + durationMs: performance.now() - stepStart, + ok: false, + }), + swallowStep, + ) const message = error instanceof Error ? error.message : String(error) const r = err([createIssue('transform_failed', `migration "${key}" threw: ${message}`)]) - onTransform?.({ from, to, path, durationMs: performance.now() - startTime, ok: false }) + safeCall( + () => + onTransform?.({ from, to, path, durationMs: performance.now() - startTime, ok: false }), + swallowTransform, + ) return r } @@ -598,7 +726,17 @@ export function createRegistry( // oxlint-disable-next-line no-await-in-loop -- intermediate validation must happen before the next migration step const result = await validateWithSchema(intermediateSchema, current, stepTo) if (!result.ok) { - onTransform?.({ from, to, path, durationMs: performance.now() - startTime, ok: false }) + safeCall( + () => + onTransform?.({ + from, + to, + path, + durationMs: performance.now() - startTime, + ok: false, + }), + swallowTransform, + ) return result } current = result.value @@ -610,12 +748,20 @@ export function createRegistry( const finalSchema = schemas[to] if (finalSchema === undefined) { const r = err([createIssue('unknown_schema', `schema "${to}" not found`)]) - onTransform?.({ from, to, path, durationMs: performance.now() - startTime, ok: false }) + safeCall( + () => + onTransform?.({ from, to, path, durationMs: performance.now() - startTime, ok: false }), + swallowTransform, + ) return r } const result = await validateWithSchema(finalSchema, current, to) if (!result.ok) { - onTransform?.({ from, to, path, durationMs: performance.now() - startTime, ok: false }) + safeCall( + () => + onTransform?.({ from, to, path, durationMs: performance.now() - startTime, ok: false }), + swallowTransform, + ) return result } current = result.value @@ -627,7 +773,10 @@ export function createRegistry( warnings: state.warnings, defaults: state.defaults, } satisfies TransformMeta) - onTransform?.({ from, to, path, durationMs: performance.now() - startTime, ok: true }) + safeCall( + () => onTransform?.({ from, to, path, durationMs: performance.now() - startTime, ok: true }), + swallowTransform, + ) return r } @@ -638,13 +787,18 @@ export function createRegistry( startTime: number, success: boolean, ): void { - onTransform?.({ - from, - to, - path: resolvedPath, - durationMs: performance.now() - startTime, - ok: success, - }) + // H2: onTransform must never escape the registry, even if a user hook throws. + safeCall( + () => + onTransform?.({ + from, + to, + path: resolvedPath, + durationMs: performance.now() - startTime, + ok: success, + }), + swallowTransform, + ) } async function transform( @@ -670,6 +824,41 @@ export function createRegistry( return r } + // B1: explicit path must be honored even when from === to. Previously the + // from===to short-circuit ran before the explicit-path branch, silently + // discarding the user-provided path. + if (options?.path !== undefined && options.path.length >= 2) { + const explicitPath = options.path + if (explicitPath[0] !== from || explicitPath.at(-1) !== to) { + const r = err([ + createIssue('invalid_input', `path must start with "${from}" and end with "${to}"`), + ]) + if (hasTiming) { + emitTransform(from, to, null, startTime, false) + } + return r + } + + // B2: validatePath defaults to true so invalid explicit paths fail + // upfront with no_path_found rather than at runtime with transform_failed. + if (options.validatePath !== false) { + const missing = validatePathMigrations(explicitPath) + if (missing !== null) { + const r = err([ + createIssue('no_path_found', `forced path is invalid: missing migration "${missing}"`), + ]) + if (hasTiming) { + emitTransform(from, to, null, startTime, false) + } + return r + } + } + + return hasTiming + ? transformInstrumented(value, from, to, explicitPath, options, startTime) + : transformCore(value, from, to, explicitPath, options) + } + if (from === to) { const validateStrategy = options?.validate ?? 'end' if (validateStrategy !== 'none') { @@ -691,53 +880,31 @@ export function createRegistry( if (hasTiming) { emitTransform(from, to, [from], startTime, true) } + // B5: return the validated value by reference; the schema is expected + // to return a normalized value, so identity aliasing is acceptable here. return ok(result.value, emptyMeta(from)) } if (hasTiming) { emitTransform(from, to, [from], startTime, true) } - return ok(value, emptyMeta(from)) + // B5: with validate:'none' we shallow-copy so a downstream mutation + // can't leak back into the caller's input. + return ok( + value === null || typeof value !== 'object' ? value : { ...(value as object) }, + emptyMeta(from), + ) } const effectiveStrategy = options?.pathStrategy ?? pathStrategy - let path: readonly Keys[] = [] - - if (options?.path !== undefined && options.path.length >= 2) { - ;({ path } = options) - - if (path[0] !== from || path.at(-1) !== to) { - const r = err([ - createIssue('invalid_input', `path must start with "${from}" and end with "${to}"`), - ]) - if (hasTiming) { - emitTransform(from, to, null, startTime, false) - } - return r - } - - if (options.validatePath === true) { - const missing = validatePathMigrations(path) - if (missing !== null) { - const r = err([ - createIssue('no_path_found', `forced path is invalid: missing migration "${missing}"`), - ]) - if (hasTiming) { - emitTransform(from, to, null, startTime, false) - } - return r - } - } - } else { - const found = resolvePath(from, to, effectiveStrategy) - if (found === null || found.length < 2) { - const r = noPathError(from, to, effectiveStrategy) - if (hasTiming) { - emitTransform(from, to, null, startTime, false) - } - return r + const found = resolvePath(from, to, effectiveStrategy) + if (found === null || found.length < 2) { + const r = noPathError(from, to, effectiveStrategy) + if (hasTiming) { + emitTransform(from, to, null, startTime, false) } - path = found + return r } + const path = found return hasTiming ? transformInstrumented(value, from, to, path, options, startTime) @@ -755,7 +922,15 @@ export function createRegistry( return validateWithSchema(schemaObj, value, schema) } - function explain(from: Keys, to: Keys): ExplainResult { + function visualize(input?: VisualizeInput): string { + return visualizeRegistry(graph, input) + } + + function explain( + from: Keys, + to: Keys, + options?: { readonly pathStrategy?: PathStrategy | undefined }, + ): ExplainResult { if (from === to) { return { from, @@ -767,7 +942,7 @@ export function createRegistry( } } - const path = resolvePath(from, to) + const path = resolvePath(from, to, options?.pathStrategy) if (path === null || path.length < 2) { const reachable = [...reachableFrom(graph, from)].toSorted() const rev = reverseGraph(graph) @@ -841,7 +1016,15 @@ export function createRegistry( // function form: call it directly, verify the key is known if (typeof identifyConfig === 'function') { - const key = identifyConfig(value) as Keys | null + // B4/I3: a throwing identify function must surface as a Result, not reject. + // oxlint-disable-next-line init-declarations -- assigned in try, used after; restructure would obscure the error path + let key: Keys | null + try { + key = identifyConfig(value) as Keys | null + } catch (error) { + const message = error instanceof Error ? error.message : String(error) + return err([createIssue('identify_failed', `identify function threw: ${message}`)]) + } if (key === null || !has(key)) { return err([createIssue('identify_failed', 'no schema matched the provided value')]) } @@ -860,7 +1043,14 @@ export function createRegistry( tryParseKeys.push(key) continue } - if ((guard as IdentifyGuard)(value)) { + // B4: a throwing guard must not crash identify. + let matched = false + try { + matched = (guard as IdentifyGuard)(value) + } catch { + matched = false + } + if (matched) { return ok(key, { schema: key }) } } @@ -875,9 +1065,23 @@ export function createRegistry( if (schema === undefined) { return null } - const validation = schema['~standard'].validate(value) - const outcome = validation instanceof Promise ? await validation : validation - return outcome.issues === undefined ? key : null + // B4: validate may throw; treat throws as "did not match". + // oxlint-disable-next-line init-declarations -- assigned in try, used after; restructure would obscure the error path + let validation: unknown + try { + validation = schema['~standard'].validate(value) + } catch { + return null + } + // T11: detect thenables, not just real Promises. + const outcome = isThenable(validation) ? await validation : validation + // T12: defensively check the result shape before reading issues. + return outcome !== null && + typeof outcome === 'object' && + !('issues' in outcome) && + 'value' in outcome + ? key + : null }), ) @@ -929,10 +1133,12 @@ export function createRegistry( transform, validate, has, - hasMigration: (from: Keys, to: Keys) => migrations.has(`${from}->${to}`), + hasMigration: (from: Keys, to: Keys) => + has(from) && has(to) && migrations.has(`${from}->${to}`), findPath: resolvePath, explain, schemas, + visualize, } if (identifyConfig !== undefined) { diff --git a/packages/doba/src/transform.ts b/packages/doba/src/transform.ts index cc79127..68d3437 100644 --- a/packages/doba/src/transform.ts +++ b/packages/doba/src/transform.ts @@ -3,6 +3,19 @@ import { type DobaIssue, createIssue } from './issue.js' import type { StandardSchemaV1 } from './standard-schema.js' import type { WarningInfo, DefaultedInfo } from './context.js' +/** + * detects real Promises *and* thenables. `instanceof Promise` misses values + * returned by libraries that produce promise-like objects without using the + * global Promise constructor (Effect, some zod configs, custom async adapters). + */ +function isThenable(value: unknown): value is PromiseLike { + return ( + value !== null && + typeof value === 'object' && + typeof (value as { then?: unknown }).then === 'function' + ) +} + /** * strategy for resolving migration paths between schemas. * - `'shortest'` finds the optimal path through the migration graph. @@ -85,22 +98,49 @@ export async function validateWithSchema( value: unknown, schemaKey: Key, ): Promise> { - const result = schema['~standard'].validate(value) - const resolved = result instanceof Promise ? await result : result + let result: unknown + try { + result = schema['~standard'].validate(value) + } catch (error) { + const message = error instanceof Error ? error.message : String(error) + return err([createIssue('validation_failed', `schema "${schemaKey}" threw: ${message}`)]) + } + + const resolved = isThenable(result) ? await result : result + + if (resolved !== null && typeof resolved === 'object' && 'issues' in resolved) { + const rawIssues = (resolved as { issues?: unknown }).issues + if (rawIssues !== undefined) { + if (!Array.isArray(rawIssues)) { + return err([ + createIssue('validation_failed', `schema "${schemaKey}" returned non-array issues`), + ]) + } + const issues: DobaIssue[] = rawIssues.map((issue) => { + const issueObj = issue as { message?: string; path?: readonly unknown[] } + const path = issueObj.path?.map((p) => + typeof p === 'object' && p !== null && 'key' in p ? (p as { key: unknown }).key : p, + ) + return createIssue( + 'validation_failed', + issueObj.message ?? 'validation failed', + path === undefined ? undefined : { path }, + ) + }) + return err(issues) + } + } - if (resolved.issues !== undefined) { - const issues: DobaIssue[] = resolved.issues.map((issue) => { - const path = issue.path?.map((p) => - typeof p === 'object' && p !== null && 'key' in p ? p.key : p, - ) - return createIssue( - 'validation_failed', - issue.message, - path === undefined ? undefined : { path }, - ) - }) - return err(issues) + if (resolved !== null && typeof resolved === 'object' && 'value' in resolved) { + return ok((resolved as { value: T }).value, { schema: schemaKey }) } - return ok(resolved.value, { schema: schemaKey }) + // malformed result (no value, no issues): treat as a validation failure + // rather than silently succeeding with undefined. + return err([ + createIssue( + 'validation_failed', + `schema "${schemaKey}" returned a malformed result (no "value" or "issues" field)`, + ), + ]) } From 72d589ed209e964114724be9d4742dc4651b9f7b Mon Sep 17 00:00:00 2001 From: Karol Broda Date: Thu, 11 Jun 2026 14:22:00 +0200 Subject: [PATCH 4/5] test(registry): add regression tests for review fixes --- packages/doba/tests/identify.test.ts | 60 +- packages/doba/tests/migration.test.ts | 12 +- packages/doba/tests/regressions.test.ts | 735 ++++++++++++++++++++++++ packages/doba/tests/transform.test.ts | 16 +- 4 files changed, 791 insertions(+), 32 deletions(-) create mode 100644 packages/doba/tests/regressions.test.ts diff --git a/packages/doba/tests/identify.test.ts b/packages/doba/tests/identify.test.ts index bc205c5..b8a9485 100644 --- a/packages/doba/tests/identify.test.ts +++ b/packages/doba/tests/identify.test.ts @@ -541,25 +541,26 @@ describe('match edge cases', () => { // ---- byField edge cases ---- describe('byField edge cases', () => { - it('numeric field values are stringified', () => { + it('numeric field values return null instead of being stringified', () => { + // T13: non-string values now return null. callers must coerce explicitly. const fn = byField('version') - expect(fn({ version: 42 })).toBe('42') + expect(fn({ version: 42 })).toBe(null) }) - it('boolean field values are stringified', () => { + it('boolean field values return null', () => { const fn = byField('active') - expect(fn({ active: true })).toBe('true') - expect(fn({ active: false })).toBe('false') + expect(fn({ active: true })).toBe(null) + expect(fn({ active: false })).toBe(null) }) - it('null field value is stringified', () => { + it('null field value returns null', () => { const fn = byField('tag') - expect(fn({ tag: null })).toBe('null') + expect(fn({ tag: null })).toBe(null) }) - it('undefined field value is stringified', () => { + it('undefined field value returns null', () => { const fn = byField('tag') - expect(fn({ tag: undefined })).toBe('undefined') + expect(fn({ tag: undefined })).toBe(null) }) it('empty prefix and suffix strings behave like no options', () => { @@ -574,14 +575,19 @@ describe('byField edge cases', () => { expect(fn({ type: 'c' })).toBe(null) }) - it('object field value is stringified via String()', () => { + it('object field value returns null instead of [object Object]', () => { const fn = byField('data') - expect(fn({ data: {} })).toBe('[object Object]') + expect(fn({ data: {} })).toBe(null) }) - it('numeric field value with prefix', () => { + it('numeric field value with prefix returns null', () => { const fn = byField('v', { prefix: 'version_' }) - expect(fn({ v: 3 })).toBe('version_3') + expect(fn({ v: 3 })).toBe(null) + }) + + it('string field values still work with prefix/suffix', () => { + const fn = byField('v', { prefix: 'version_' }) + expect(fn({ v: '3' })).toBe('version_3') }) }) @@ -1110,10 +1116,10 @@ describe('firstMatch composition', () => { }) }) -// ---- type coercion in byField ---- +// ---- byField rejects non-string field values (T13) ---- -describe('type coercion in byField', () => { - it('byField where field value is a number gets String()-ed', async () => { +describe('byField rejects non-string field values', () => { + it('byField where field value is a number returns null', async () => { type N1 = { version: number; data: string } const n1Schema = createMockSchema((v) => { const obj = v as Record @@ -1129,25 +1135,25 @@ describe('type coercion in byField', () => { identify: byField('version'), }) + // T13: numeric field value no longer stringifies to "1"; returns null + // because the field value isn't a string. callers must coerce explicitly. const result = await registry.identify({ version: 1, data: 'hello' }) - expect(result.ok).toBe(true) - if (result.ok) { - expect(result.value).toBe('1') - } + expect(result.ok).toBe(false) }) - it('byField where field value is an object stringifies to [object Object]', () => { + it('byField where field value is an object returns null', () => { const fn = byField('tag') - const result = fn({ tag: { nested: true } }) - expect(result).toBe('[object Object]') + expect(fn({ tag: { nested: true } })).toBe(null) }) - it('byField with map still stringifies the field before lookup', () => { + it('byField with map rejects non-string field values', () => { const fn = byField('version', { map: { '2': 'v2', '3': 'v3' } }) - // numeric field value gets String()-ed, then looked up in map - expect(fn({ version: 2 })).toBe('v2') - expect(fn({ version: 3 })).toBe('v3') + // non-string values return null before the map is consulted + expect(fn({ version: 2 })).toBe(null) + expect(fn({ version: 3 })).toBe(null) expect(fn({ version: 99 })).toBe(null) + // string values still consult the map + expect(fn({ version: '2' })).toBe('v2') }) }) diff --git a/packages/doba/tests/migration.test.ts b/packages/doba/tests/migration.test.ts index d4949e1..b4a3092 100644 --- a/packages/doba/tests/migration.test.ts +++ b/packages/doba/tests/migration.test.ts @@ -71,21 +71,25 @@ describe('resolveMigrations', () => { expect(resolved.size).toBe(0) }) - it('skips object missing migrate field', () => { - const { migrations: resolved } = resolveMigrations({ + it('skips object missing migrate field (with warning)', () => { + const { migrations: resolved, warnings } = resolveMigrations({ 'a->b': { notMigrate: (v: unknown) => v }, }) expect(resolved.size).toBe(0) + // T5: skipped defs now emit a warning instead of failing silently. + expect(warnings.some((w) => w.message.includes('a->b'))).toBe(true) }) - it('skips reversible missing forward/backward', () => { - const { migrations: resolved } = resolveMigrations({ + it('skips reversible missing forward/backward (with warning)', () => { + const { migrations: resolved, warnings } = resolveMigrations({ 'a<->b': { forward: (v: unknown) => v }, 'c<->d': { backward: (v: unknown) => v }, 'e<->f': { migrate: (v: unknown) => v }, 'g<->h': 'not an object', }) expect(resolved.size).toBe(0) + // T5: all four malformed reversible defs warn. + expect(warnings.length).toBeGreaterThanOrEqual(3) }) it('one-way overrides reversible forward with warning', () => { diff --git a/packages/doba/tests/regressions.test.ts b/packages/doba/tests/regressions.test.ts new file mode 100644 index 0000000..8962adc --- /dev/null +++ b/packages/doba/tests/regressions.test.ts @@ -0,0 +1,735 @@ +/** + * Regression tests for bugs found during the full review. Each describe + * block is keyed by the bug ID from the review (H1-H5, B1-B5, T1-T13) and + * pins down the fixed behavior so the bugs can't be reintroduced silently. + */ +/* oxlint-disable unicorn/no-thenable -- the T11 regression test deliberately constructs a thenable */ +import { describe, it, expect, vi } from 'vitest' +import { createRegistry, byField, match, tryParse, type TransformHookInfo } from '../src/index.js' +import { MigrationConflictError, resolveMigrations, DEFAULT_COST } from '../src/migration.js' +import { createMockSchema, userSchemas, sampleDatabaseUser, dynamicMigrations } from './helpers.js' + +// ============================================================ +// helpers +// ============================================================ + +/** + * builds a thenable (NOT a real Promise) that resolves to `value`. used to + * verify T11: the registry must await thenables returned by migrations, not + * just values that are `instanceof Promise`. + */ +function makeThenable(value: T): unknown { + return { + then(resolve: (v: T) => void): void { + resolve(value) + }, + } +} + +// ============================================================ +// H1/H2/H3/H5 — hook throws must not escape transform +// ============================================================ + +describe('H1-H5: throwing hooks surface as Result errors, not rejections', () => { + it('onStep throw does not reject; onTransform still fires', async () => { + const transformCalls: TransformHookInfo[] = [] + const reg = createRegistry({ + schemas: { + a: createMockSchema<{ a: number }>((v) => ({ ok: true, value: v as { a: number } })), + b: createMockSchema<{ b: number }>((v) => ({ ok: true, value: v as { b: number } })), + c: createMockSchema<{ c: number }>((v) => ({ ok: true, value: v as { c: number } })), + }, + migrations: { + 'a->b': (v: { a: number }) => ({ b: v.a }), + 'b->c': (v: { b: number }) => ({ c: v.b }), + }, + hooks: { + onStep: () => { + throw new Error('step boom') + }, + onTransform: (info) => transformCalls.push(info), + }, + }) + + // must NOT reject + const r = await reg.transform({ a: 1 }, 'a', 'c', { validate: 'none' }) + expect(r.ok).toBe(true) + // onTransform still fired despite onStep throwing + expect(transformCalls).toHaveLength(1) + expect(transformCalls[0]?.ok).toBe(true) + }) + + it('onTransform throw does not reject the call', async () => { + const reg = createRegistry({ + schemas: userSchemas, + migrations: { + 'database->frontend': (u) => ({ + id: u.id, + email: u.email, + createdAt: u.createdAt, + role: u.role, + }), + }, + hooks: { + onTransform: () => { + throw new Error('transform boom') + }, + }, + }) + + const r = await reg.transform(sampleDatabaseUser, 'database', 'frontend') + expect(r.ok).toBe(true) + }) + + it('onWarning throw via deprecated migration does not reject', async () => { + const reg = createRegistry({ + schemas: userSchemas, + migrations: { + 'database->frontend': { + migrate: (u) => ({ + id: u.id, + email: u.email, + createdAt: u.createdAt, + role: u.role, + }), + deprecated: 'old', + }, + }, + hooks: { + onWarning: () => { + throw new Error('warn boom') + }, + }, + }) + + const r = await reg.transform(sampleDatabaseUser, 'database', 'frontend') + expect(r.ok).toBe(true) + }) + + it('onWarning throw via ctx.warn is absorbed (presented as transform_failed in old code, now absorbed by safeCall)', async () => { + const reg = createRegistry({ + schemas: userSchemas, + migrations: { + 'database->frontend': (u, ctx) => { + ctx.warn('user warning') + return { + id: u.id, + email: u.email, + createdAt: u.createdAt, + role: u.role, + } + }, + }, + hooks: { + onWarning: () => { + throw new Error('ctx warn boom') + }, + }, + }) + + // ctx.warn throwing is absorbed by the safeCall wrapper around the + // onWarning invocation passed to createTransformContext. the migration + // still completes successfully. + const r = await reg.transform(sampleDatabaseUser, 'database', 'frontend') + expect(r.ok).toBe(true) + }) + + it('debug mode logs swallowed hook errors without rejecting', async () => { + const spy = vi.spyOn(console, 'log').mockImplementation(() => {}) + const reg = createRegistry({ + schemas: userSchemas, + migrations: { + 'database->frontend': (u) => ({ + id: u.id, + email: u.email, + createdAt: u.createdAt, + role: u.role, + }), + }, + debug: true, + hooks: { + onTransform: () => { + throw new Error('swallowed boom') + }, + }, + }) + + const r = await reg.transform(sampleDatabaseUser, 'database', 'frontend') + expect(r.ok).toBe(true) + const logs = spy.mock.calls.map((c) => c[0] as string) + expect(logs.some((l) => l.includes('swallowed'))).toBe(true) + spy.mockRestore() + }) +}) + +// ============================================================ +// B1 — explicit path honored when from === to +// ============================================================ + +describe('B1: explicit path honored when from === to', () => { + it('routes through the explicit path instead of returning identity', async () => { + const warnings: string[] = [] + const reg = createRegistry({ + schemas: { + a: createMockSchema<{ a: number }>((v) => ({ ok: true, value: v as { a: number } })), + b: createMockSchema<{ a: number }>((v) => ({ ok: true, value: v as { a: number } })), + }, + migrations: { + 'a->b': (v) => v, + 'b->a': (v) => v, + }, + hooks: { onWarning: (m) => warnings.push(m) }, + }) + + const r = await reg.transform({ a: 1 }, 'a', 'a', { + path: ['a', 'b', 'a'], + }) + expect(r.ok).toBe(true) + if (r.ok) { + expect(r.meta.path).toEqual(['a', 'b', 'a']) + expect(r.meta.steps).toHaveLength(2) + } + }) + + it('still returns identity when no explicit path is given', async () => { + const reg = createRegistry({ + schemas: { + a: createMockSchema<{ a: number }>((v) => ({ ok: true, value: v as { a: number } })), + }, + migrations: {}, + }) + const r = await reg.transform({ a: 1 }, 'a', 'a') + expect(r.ok).toBe(true) + if (r.ok) { + expect(r.meta.path).toEqual(['a']) + } + }) +}) + +// ============================================================ +// B2 — validatePath defaults to true +// ============================================================ + +describe('B2: validatePath defaults to true', () => { + it('rejects invalid explicit path upfront with no_path_found', async () => { + const reg = createRegistry({ + schemas: { + a: createMockSchema<{ a: number }>((v) => ({ ok: true, value: v as { a: number } })), + b: createMockSchema<{ b: number }>((v) => ({ ok: true, value: v as { b: number } })), + }, + migrations: { 'a->b': (v: { a: number }) => ({ b: v.a }) }, + }) + const r = await reg.transform({ a: 1 }, 'a', 'b', { + path: ['a', 'b', 'a', 'b'], + }) + expect(r.ok).toBe(false) + if (!r.ok) { + expect(r.issues[0]?.code).toBe('no_path_found') + } + }) + + it('can be disabled with validatePath: false', async () => { + const reg = createRegistry({ + schemas: { + a: createMockSchema<{ a: number }>((v) => ({ ok: true, value: v as { a: number } })), + b: createMockSchema<{ b: number }>((v) => ({ ok: true, value: v as { b: number } })), + }, + migrations: { 'a->b': (v: { a: number }) => ({ b: v.a }) }, + }) + const r = await reg.transform({ a: 1 }, 'a', 'b', { + path: ['a', 'b', 'a', 'b'], + validatePath: false, + }) + expect(r.ok).toBe(false) + if (!r.ok) { + expect(r.issues[0]?.code).toBe('transform_failed') + } + }) +}) + +// ============================================================ +// B4 / I3 — identify() handles throwing schemas and guards +// ============================================================ + +describe('B4/I3: identify handles throwing inputs as Result errors', () => { + it('identify function that throws returns err, not rejection', async () => { + const reg = createRegistry({ + schemas: { + a: createMockSchema<{ a: number }>((v) => ({ ok: true, value: v as { a: number } })), + }, + migrations: {}, + identify: () => { + throw new Error('identify boom') + }, + }) + + const r = await reg.identify({ a: 1 }) + expect(r.ok).toBe(false) + if (!r.ok) { + expect(r.issues[0]?.code).toBe('identify_failed') + expect(r.issues[0]?.message).toContain('identify boom') + } + }) + + it('tryParse schema whose validate throws is treated as no match', async () => { + const throwingSchema = { + '~standard': { + version: 1 as const, + vendor: 'throwing', + validate: () => { + throw new Error('schema boom') + }, + }, + } + const okSchema = createMockSchema<{ x: number }>((v) => ({ + ok: true, + value: v as { x: number }, + })) + const reg = createRegistry({ + schemas: { good: okSchema, bad: throwingSchema }, + migrations: {}, + identify: { + good: match.field('x'), + bad: tryParse, + }, + }) + + // must not reject + const r = await reg.identify({ x: 1 }) + expect(r.ok).toBe(true) + if (r.ok) { + expect(r.value).toBe('good') + } + }) + + it('guard map guard that throws is treated as no match', async () => { + const reg = createRegistry({ + schemas: { + a: createMockSchema<{ a: number }>((v) => ({ ok: true, value: v as { a: number } })), + b: createMockSchema<{ b: number }>((v) => ({ ok: true, value: v as { b: number } })), + }, + migrations: {}, + identify: { + a: () => { + throw new Error('guard boom') + }, + b: match.field('b'), + }, + }) + + const r = await reg.identify({ b: 1 }) + expect(r.ok).toBe(true) + if (r.ok) { + expect(r.value).toBe('b') + } + }) +}) + +// ============================================================ +// B5 — from===to with validate:'none' returns a shallow copy +// ============================================================ + +describe('B5: identity transform does not return input by reference', () => { + it('from===to with validate:none returns a shallow copy', async () => { + const input = { a: 1 } + const reg = createRegistry({ + schemas: { + a: createMockSchema<{ a: number }>((v) => ({ ok: true, value: v as { a: number } })), + }, + migrations: {}, + }) + const r = await reg.transform(input, 'a', 'a', { validate: 'none' }) + expect(r.ok).toBe(true) + if (r.ok) { + expect(r.value).toEqual(input) + expect(r.value).not.toBe(input) + } + }) +}) + +// ============================================================ +// T1 — hasMigration false for unknown schemas +// ============================================================ + +describe('T1: hasMigration rejects unknown schema endpoints', () => { + it('returns false when from is not a registered schema', () => { + const reg = createRegistry({ + schemas: { + a: createMockSchema<{ a: number }>((v) => ({ ok: true, value: v as { a: number } })), + }, + migrations: {}, + }) + // @ts-expect-error testing runtime behavior + expect(reg.hasMigration('nonexistent', 'a')).toBe(false) + }) + + it('returns false when to is not a registered schema', () => { + const reg = createRegistry({ + schemas: { + a: createMockSchema<{ a: number }>((v) => ({ ok: true, value: v as { a: number } })), + }, + migrations: {}, + }) + // @ts-expect-error testing runtime behavior + expect(reg.hasMigration('a', 'nonexistent')).toBe(false) + }) +}) + +// ============================================================ +// T2 — arrow-bearing migration keys are rejected +// ============================================================ + +describe('T2: malformed migration keys are rejected with a warning', () => { + it('rejects "a->b->c" (multiple arrows)', () => { + const { migrations, warnings } = resolveMigrations({ + 'a->b->c': (v: unknown) => v, + }) + expect(migrations.size).toBe(0) + expect(warnings.some((w) => w.message.includes('malformed key'))).toBe(true) + }) + + it('rejects reversible keys with extra arrows', () => { + const { migrations, warnings } = resolveMigrations({ + 'a<->b<->c': { + forward: (v: unknown) => v, + backward: (v: unknown) => v, + }, + }) + expect(migrations.size).toBe(0) + expect(warnings.some((w) => w.message.includes('malformed key'))).toBe(true) + }) +}) + +// ============================================================ +// T3 — validate:'each' validates the input schema +// ============================================================ + +describe('T3: validate each validates the input schema', () => { + it('rejects malformed input with validate:each', async () => { + const aSchema = createMockSchema<{ a: number }>((v) => { + if (typeof (v as { a?: unknown })?.a !== 'number') { + return { ok: false, message: 'a must be number' } + } + return { ok: true, value: v as { a: number } } + }) + const bSchema = createMockSchema<{ b: number }>((v) => ({ + ok: true, + value: v as { b: number }, + })) + const reg = createRegistry({ + schemas: { a: aSchema, b: bSchema }, + migrations: { + 'a->b': (v: { a: number }) => ({ b: v.a }), + }, + }) + + const r = await reg.transform( + // @ts-expect-error intentionally bad input + { totallyWrong: true }, + 'a', + 'b', + { validate: 'each' }, + ) + expect(r.ok).toBe(false) + if (!r.ok) { + expect(r.issues[0]?.code).toBe('validation_failed') + } + }) +}) + +// ============================================================ +// T5 — malformed migration defs emit warnings (not silent skips) +// ============================================================ + +describe('T5: skipped migration definitions emit warnings', () => { + it('warns when a reversible migration is missing backward', () => { + const { warnings } = resolveMigrations({ + 'a<->b': { forward: (v: unknown) => v }, + }) + expect(warnings.some((w) => w.message.includes('"forward" and "backward"'))).toBe(true) + }) + + it('warns when pipe callback returns non-function', () => { + const { warnings } = resolveMigrations({ + 'a->b': { pipe: () => 'not a function' as unknown }, + }) + expect(warnings.some((w) => w.message.includes('pipe callback must return a function'))).toBe( + true, + ) + }) + + it('warns when migrate is not a function', () => { + const { warnings } = resolveMigrations({ + 'a->b': { migrate: 'not a function' }, + }) + expect(warnings.some((w) => w.message.includes('"migrate" must be a function'))).toBe(true) + }) +}) + +// ============================================================ +// T6 — MigrationConflictError is a typed error +// ============================================================ + +describe('T6: MigrationConflictError is typed', () => { + it('throws a MigrationConflictError with structured fields', () => { + try { + resolveMigrations({ + 'a<->c': { forward: (v: unknown) => v, backward: (v: unknown) => v }, + 'c<->a': { forward: (v: unknown) => v, backward: (v: unknown) => v }, + }) + throw new Error('should have thrown') + } catch (error) { + expect(error).toBeInstanceOf(MigrationConflictError) + const err = error as MigrationConflictError + expect(err.edge).toBe('c->a') + expect(err.sources).toEqual(['a<->c', 'c<->a']) + expect(err.name).toBe('MigrationConflictError') + } + }) +}) + +// ============================================================ +// T8 — cost: NaN/Infinity/negative are rejected +// ============================================================ + +describe('T8: non-finite or negative costs fall back to default', () => { + it('NaN cost falls back to DEFAULT_COST', () => { + const { migrations } = resolveMigrations({ + 'a->b': { migrate: (v: unknown) => v, cost: NaN }, + }) + expect(migrations.get('a->b')?.cost).toBe(DEFAULT_COST) + }) + + it('Infinity cost falls back to DEFAULT_COST', () => { + const { migrations } = resolveMigrations({ + 'a->b': { migrate: (v: unknown) => v, cost: Infinity }, + }) + expect(migrations.get('a->b')?.cost).toBe(DEFAULT_COST) + }) + + it('negative cost falls back to DEFAULT_COST', () => { + const { migrations } = resolveMigrations({ + 'a->b': { migrate: (v: unknown) => v, cost: -5 }, + }) + expect(migrations.get('a->b')?.cost).toBe(DEFAULT_COST) + }) + + it('finite non-negative cost is honored', () => { + const { migrations } = resolveMigrations({ + 'a->b': { migrate: (v: unknown) => v, cost: 0 }, + }) + expect(migrations.get('a->b')?.cost).toBe(0) + }) +}) + +// ============================================================ +// T9 — pipe.add() object/array defaults are cloned per call +// ============================================================ + +describe('T9: pipe.add() defaults are not shared across transforms', () => { + it('array default is fresh on each call', async () => { + const reg = createRegistry({ + schemas: { + a: createMockSchema<{ a: number }>((v) => ({ ok: true, value: v as { a: number } })), + b: createMockSchema<{ a: number; list: number[] }>((v) => ({ + ok: true, + value: v as { a: number; list: number[] }, + })), + }, + migrations: dynamicMigrations({ + 'a->b': { + pipe: (p: { add: (n: string, v: unknown) => unknown }) => p.add('list', [1, 2, 3]), + }, + }), + }) + + const r1 = await reg.transform({ a: 1 }, 'a', 'b', { validate: 'none' }) + if (r1.ok) { + r1.value.list.push(999) + } + const r2 = await reg.transform({ a: 2 }, 'a', 'b', { validate: 'none' }) + expect(r2.ok).toBe(true) + if (r2.ok) { + expect(r2.value.list).toEqual([1, 2, 3]) + } + }) + + it('object default is fresh on each call', async () => { + const reg = createRegistry({ + schemas: { + a: createMockSchema<{ a: number }>((v) => ({ ok: true, value: v as { a: number } })), + b: createMockSchema<{ a: number; meta: { tag: string } }>((v) => ({ + ok: true, + value: v as { a: number; meta: { tag: string } }, + })), + }, + migrations: dynamicMigrations({ + 'a->b': { + pipe: (p: { add: (n: string, v: unknown) => unknown }) => p.add('meta', { tag: 'x' }), + }, + }), + }) + + const r1 = await reg.transform({ a: 1 }, 'a', 'b', { validate: 'none' }) + if (r1.ok) { + r1.value.meta.tag = 'MUTATED' + } + const r2 = await reg.transform({ a: 2 }, 'a', 'b', { validate: 'none' }) + expect(r2.ok).toBe(true) + if (r2.ok) { + expect(r2.value.meta.tag).toBe('x') + } + }) +}) + +// ============================================================ +// T10 — ctx.defaulted() handles Symbol path entries +// ============================================================ + +describe('T10: ctx.defaulted() handles Symbol path entries', () => { + it('does not throw when path contains a Symbol', async () => { + const sym = Symbol('s') + const reg = createRegistry({ + schemas: { + a: createMockSchema<{ a: number }>((v) => ({ ok: true, value: v as { a: number } })), + b: createMockSchema<{ a: number; b: number }>((v) => ({ + ok: true, + value: v as { a: number; b: number }, + })), + }, + migrations: { + 'a->b': (v, ctx) => { + ctx.defaulted([0, sym, 'x'], 'mixed keys') + return { ...v, b: 1 } + }, + }, + }) + + const r = await reg.transform({ a: 1 }, 'a', 'b', { validate: 'none' }) + expect(r.ok).toBe(true) + if (r.ok) { + expect(r.meta.defaults[0]?.message).toBe('mixed keys') + } + }) +}) + +// ============================================================ +// T11 — thenable returns are awaited +// ============================================================ + +describe('T11: thenable returns are awaited', () => { + it('migration returning a thenable is awaited correctly', async () => { + const reg = createRegistry({ + schemas: { + a: createMockSchema<{ a: number }>((v) => ({ ok: true, value: v as { a: number } })), + b: createMockSchema<{ b: number }>((v) => ({ ok: true, value: v as { b: number } })), + }, + migrations: dynamicMigrations({ + 'a->b': () => makeThenable({ b: 42 }), + }), + }) + + const r = await reg.transform({ a: 1 }, 'a', 'b', { validate: 'none' }) + expect(r.ok).toBe(true) + if (r.ok) { + expect(r.value).toEqual({ b: 42 }) + } + }) +}) + +// ============================================================ +// T12 — malformed schema results do not crash or silently succeed +// ============================================================ + +describe('T12: malformed schema results are rejected', () => { + it('result with no value or issues is rejected', async () => { + const malformedSchema = { + '~standard': { + version: 1 as const, + vendor: 'malformed', + validate: () => ({}) as unknown, + }, + } + const reg = createRegistry({ + schemas: { + a: createMockSchema<{ a: number }>((v) => ({ ok: true, value: v as { a: number } })), + b: malformedSchema as never, + }, + migrations: dynamicMigrations({ + 'a->b': (v: { a: number }) => ({ ...v }), + }), + }) + + const r = await reg.transform({ a: 1 }, 'a', 'b') + expect(r.ok).toBe(false) + if (!r.ok) { + expect(r.issues[0]?.code).toBe('validation_failed') + expect(r.issues[0]?.message).toContain('malformed') + } + }) + + it('schema.validate that throws is rejected, not crashed', async () => { + const throwingSchema = { + '~standard': { + version: 1 as const, + vendor: 'throwing', + validate: () => { + throw new Error('schema boom') + }, + }, + } + const reg = createRegistry({ + schemas: { a: throwingSchema as never }, + migrations: {}, + }) + + const r = await reg.validate({ x: 1 }, 'a') + expect(r.ok).toBe(false) + if (!r.ok) { + expect(r.issues[0]?.code).toBe('validation_failed') + expect(r.issues[0]?.message).toContain('schema boom') + } + }) +}) + +// ============================================================ +// T13 — byField rejects non-string field values (covered in identify.test.ts) +// ============================================================ + +describe('T13: byField rejects non-string values', () => { + it('returns null for object field value', () => { + const fn = byField('tag') + expect(fn({ tag: { x: 1 } })).toBe(null) + }) + + it('returns null for numeric field value', () => { + const fn = byField('version') + expect(fn({ version: 1 })).toBe(null) + }) +}) + +// ============================================================ +// A5 — explain accepts pathStrategy override +// ============================================================ + +describe('A5: explain accepts pathStrategy override', () => { + it('explain with direct pathStrategy reports no path for indirect routes', () => { + const reg = createRegistry({ + schemas: { + a: createMockSchema<{ a: number }>((v) => ({ ok: true, value: v as { a: number } })), + b: createMockSchema<{ b: number }>((v) => ({ ok: true, value: v as { b: number } })), + c: createMockSchema<{ c: number }>((v) => ({ ok: true, value: v as { c: number } })), + }, + migrations: { + 'a->b': (v: { a: number }) => ({ b: v.a }), + 'b->c': (v: { b: number }) => ({ c: v.b }), + }, + }) + + const short = reg.explain('a', 'c') + expect(short.path).toEqual(['a', 'b', 'c']) + + const direct = reg.explain('a', 'c', { pathStrategy: 'direct' }) + expect(direct.path).toBe(null) + }) +}) diff --git a/packages/doba/tests/transform.test.ts b/packages/doba/tests/transform.test.ts index ad72fd6..125b685 100644 --- a/packages/doba/tests/transform.test.ts +++ b/packages/doba/tests/transform.test.ts @@ -221,11 +221,25 @@ describe('transform (path options)', () => { } }) - it('fails at runtime without validatePath for invalid explicit path', async () => { + it('fails upfront for invalid explicit path (validatePath defaults to true)', async () => { const result = await registry.transform(sampleDatabaseUser, 'database', 'ai', { path: ['database', 'legacy', 'ai'], }) expect(result.ok).toBe(false) + if (!result.ok) { + // B2: validatePath now defaults to true, so the path is rejected upfront + // with no_path_found instead of failing at runtime with transform_failed. + expect(result.issues[0]?.code).toBe('no_path_found') + expect(result.issues[0]?.message).toContain('database->legacy') + } + }) + + it('fails at runtime when validatePath is explicitly disabled', async () => { + const result = await registry.transform(sampleDatabaseUser, 'database', 'ai', { + path: ['database', 'legacy', 'ai'], + validatePath: false, + }) + expect(result.ok).toBe(false) if (!result.ok) { expect(result.issues[0]?.code).toBe('transform_failed') expect(result.issues[0]?.message).toContain('missing migration') From 9e149eacab23e66397ff572177c2db2f6361fedf Mon Sep 17 00:00:00 2001 From: Karol Broda Date: Thu, 11 Jun 2026 15:02:00 +0200 Subject: [PATCH 5/5] refactor(registry): merge transformCore and transformInstrumented into runTransform --- packages/doba/src/registry.ts | 281 +++++++++++----------------------- 1 file changed, 90 insertions(+), 191 deletions(-) diff --git a/packages/doba/src/registry.ts b/packages/doba/src/registry.ts index 08d35b3..3ac0e48 100644 --- a/packages/doba/src/registry.ts +++ b/packages/doba/src/registry.ts @@ -486,120 +486,56 @@ export function createRegistry( ]) } - async function transformCore( - value: unknown, - _from: Keys, - to: Keys, - path: readonly Keys[], - options: TransformOptions | undefined, - ): Promise> { - const state = createTransformState() - const steps: StepInfo[] = [] - const validateStrategy = options?.validate ?? 'end' - const totalSteps = path.length - 1 - let current = value - - // T3: 'each' validates intermediate schemas; the input schema (path[0]) - // is validated here so a malformed input doesn't pass through unchecked. - if (validateStrategy === 'each' && totalSteps > 0) { - const inputSchema = schemas[path[0] as Keys] - if (inputSchema !== undefined) { - const result = await validateWithSchema(inputSchema, current, path[0] as Keys) - if (!result.ok) { - return result - } - current = result.value - } - } - - for (let i = 0; i < totalSteps; i++) { - const stepFrom = path[i] as Keys - const stepTo = path[i + 1] as Keys - const key = `${stepFrom}->${stepTo}` - const migration = migrations.get(key) - - if (migration === undefined) { - return err([createIssue('transform_failed', `missing migration "${key}"`)]) - } - - steps.push({ - from: stepFrom, - to: stepTo, - label: migration.label, - deprecated: migration.deprecated === false ? undefined : migration.deprecated, - }) + /** + * optional timing/hook instrumentation. when undefined (the common + * no-hooks case), all performance.now() calls and per-step allocations + * are skipped, matching the original fast-path behavior. + */ + type Instrumentation = { + readonly startTime: number + readonly emitStep: (info: { + readonly from: Keys + readonly to: Keys + readonly index: number + readonly total: number + readonly label: string | undefined + readonly durationMs: number + readonly ok: boolean + }) => void + readonly emitTransform: (succeeded: boolean, resolvedPath: readonly Keys[] | null) => void + } - if (migration.deprecated !== false) { - let reason = '' - if (typeof migration.deprecated === 'string' && migration.deprecated !== 'deprecated') { - reason = `: ${migration.deprecated}` - } - state.warnings.push({ - message: `using deprecated migration "${key}"${reason}`, - from: stepFrom, - to: stepTo, - }) - // H5: deprecated-warning invocation must not escape the registry. + function makeInstrumentation(from: Keys, to: Keys, startTime: number): Instrumentation { + return { + startTime, + emitStep: (info) => safeCall(() => onStep?.(info), swallowStep), + emitTransform: (succeeded, resolvedPath) => safeCall( - () => onWarning?.(`using deprecated migration "${key}"${reason}`, stepFrom, stepTo), - swallowWarning, - ) - } - - const ctx = createTransformContext(state, stepFrom, stepTo, (msg, f, t) => - safeCall(() => onWarning?.(msg, f, t), swallowWarning), - ) - - try { - const result = migration.fn(current, ctx) - // T11: detect thenables, not just real Promises. - // oxlint-disable-next-line no-await-in-loop -- migration steps must run sequentially, each depends on the previous result - current = isThenable(result) ? await result : (result as unknown) - } catch (error) { - const message = error instanceof Error ? error.message : String(error) - return err([createIssue('transform_failed', `migration "${key}" threw: ${message}`)]) - } - - if (validateStrategy === 'each' && i < totalSteps - 1) { - const intermediateSchema = schemas[stepTo] - if (intermediateSchema !== undefined) { - // oxlint-disable-next-line no-await-in-loop -- intermediate validation must happen before the next migration step - const result = await validateWithSchema(intermediateSchema, current, stepTo) - if (!result.ok) { - return result - } - current = result.value - } - } - } - - if (validateStrategy !== 'none') { - const finalSchema = schemas[to] - if (finalSchema === undefined) { - return err([createIssue('unknown_schema', `schema "${to}" not found`)]) - } - const result = await validateWithSchema(finalSchema, current, to) - if (!result.ok) { - return result - } - current = result.value + () => + onTransform?.({ + from, + to, + path: resolvedPath, + durationMs: performance.now() - startTime, + ok: succeeded, + }), + swallowTransform, + ), } - - return ok(current, { - path, - steps, - warnings: state.warnings, - defaults: state.defaults, - } satisfies TransformMeta) } - async function transformInstrumented( + /** + * single transform implementation. `instrumentation` is undefined when no + * timing hooks are registered, in which case all performance.now() calls + * and per-step object allocations are skipped (the documented fast path). + */ + async function runTransform( value: unknown, - from: Keys, + _from: Keys, to: Keys, path: readonly Keys[], options: TransformOptions | undefined, - startTime: number, + instrumentation: Instrumentation | undefined, ): Promise> { const state = createTransformState() const steps: StepInfo[] = [] @@ -614,17 +550,7 @@ export function createRegistry( if (inputSchema !== undefined) { const result = await validateWithSchema(inputSchema, current, path[0] as Keys) if (!result.ok) { - safeCall( - () => - onTransform?.({ - from, - to, - path, - durationMs: performance.now() - startTime, - ok: false, - }), - swallowTransform, - ) + instrumentation?.emitTransform(false, path) return result } current = result.value @@ -638,14 +564,8 @@ export function createRegistry( const migration = migrations.get(key) if (migration === undefined) { - const r = err([createIssue('transform_failed', `missing migration "${key}"`)]) - // H2: onTransform invocations must not escape the registry. - safeCall( - () => - onTransform?.({ from, to, path, durationMs: performance.now() - startTime, ok: false }), - swallowTransform, - ) - return r + instrumentation?.emitTransform(false, path) + return err([createIssue('transform_failed', `missing migration "${key}"`)]) } steps.push({ @@ -675,7 +595,7 @@ export function createRegistry( const ctx = createTransformContext(state, stepFrom, stepTo, (msg, f, t) => safeCall(() => onWarning?.(msg, f, t), swallowWarning), ) - const stepStart = performance.now() + const stepStart = instrumentation !== undefined ? performance.now() : 0 try { const result = migration.fn(current, ctx) @@ -683,41 +603,32 @@ export function createRegistry( // oxlint-disable-next-line no-await-in-loop -- migration steps must run sequentially, each depends on the previous result current = isThenable(result) ? await result : (result as unknown) // H1: onStep invocations must not escape the registry. - safeCall( - () => - onStep?.({ - from: stepFrom, - to: stepTo, - index: i, - total: totalSteps, - label: migration.label, - durationMs: performance.now() - stepStart, - ok: true, - }), - swallowStep, - ) + if (instrumentation !== undefined) { + instrumentation.emitStep({ + from: stepFrom, + to: stepTo, + index: i, + total: totalSteps, + label: migration.label, + durationMs: performance.now() - stepStart, + ok: true, + }) + } } catch (error) { - safeCall( - () => - onStep?.({ - from: stepFrom, - to: stepTo, - index: i, - total: totalSteps, - label: migration.label, - durationMs: performance.now() - stepStart, - ok: false, - }), - swallowStep, - ) + if (instrumentation !== undefined) { + instrumentation.emitStep({ + from: stepFrom, + to: stepTo, + index: i, + total: totalSteps, + label: migration.label, + durationMs: performance.now() - stepStart, + ok: false, + }) + } const message = error instanceof Error ? error.message : String(error) - const r = err([createIssue('transform_failed', `migration "${key}" threw: ${message}`)]) - safeCall( - () => - onTransform?.({ from, to, path, durationMs: performance.now() - startTime, ok: false }), - swallowTransform, - ) - return r + instrumentation?.emitTransform(false, path) + return err([createIssue('transform_failed', `migration "${key}" threw: ${message}`)]) } if (validateStrategy === 'each' && i < totalSteps - 1) { @@ -726,17 +637,7 @@ export function createRegistry( // oxlint-disable-next-line no-await-in-loop -- intermediate validation must happen before the next migration step const result = await validateWithSchema(intermediateSchema, current, stepTo) if (!result.ok) { - safeCall( - () => - onTransform?.({ - from, - to, - path, - durationMs: performance.now() - startTime, - ok: false, - }), - swallowTransform, - ) + instrumentation?.emitTransform(false, path) return result } current = result.value @@ -747,21 +648,12 @@ export function createRegistry( if (validateStrategy !== 'none') { const finalSchema = schemas[to] if (finalSchema === undefined) { - const r = err([createIssue('unknown_schema', `schema "${to}" not found`)]) - safeCall( - () => - onTransform?.({ from, to, path, durationMs: performance.now() - startTime, ok: false }), - swallowTransform, - ) - return r + instrumentation?.emitTransform(false, path) + return err([createIssue('unknown_schema', `schema "${to}" not found`)]) } const result = await validateWithSchema(finalSchema, current, to) if (!result.ok) { - safeCall( - () => - onTransform?.({ from, to, path, durationMs: performance.now() - startTime, ok: false }), - swallowTransform, - ) + instrumentation?.emitTransform(false, path) return result } current = result.value @@ -773,10 +665,7 @@ export function createRegistry( warnings: state.warnings, defaults: state.defaults, } satisfies TransformMeta) - safeCall( - () => onTransform?.({ from, to, path, durationMs: performance.now() - startTime, ok: true }), - swallowTransform, - ) + instrumentation?.emitTransform(true, path) return r } @@ -854,9 +743,14 @@ export function createRegistry( } } - return hasTiming - ? transformInstrumented(value, from, to, explicitPath, options, startTime) - : transformCore(value, from, to, explicitPath, options) + return runTransform( + value, + from, + to, + explicitPath, + options, + hasTiming ? makeInstrumentation(from, to, startTime) : undefined, + ) } if (from === to) { @@ -906,9 +800,14 @@ export function createRegistry( } const path = found - return hasTiming - ? transformInstrumented(value, from, to, path, options, startTime) - : transformCore(value, from, to, path, options) + return runTransform( + value, + from, + to, + path, + options, + hasTiming ? makeInstrumentation(from, to, startTime) : undefined, + ) } function validate(value: unknown, schema: Keys): Promise> {