From 27a4fddfafaab926d431421cf4b7a4f09d5b75a3 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 29 Aug 2026 17:19:36 +0000 Subject: [PATCH 1/2] Make SQL types portable across package instances Strip implementation-only query-builder details from published declarations, infer decoder results structurally, and compile two physical package copies to prevent regressions. --- drizzle-orm/src/sql/sql.ts | 5 +- drizzle-orm/tests/exports-resolution.test.ts | 62 ++++++++++++++++++++ 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/drizzle-orm/src/sql/sql.ts b/drizzle-orm/src/sql/sql.ts index b4d0b1571c..122a6ebc38 100644 --- a/drizzle-orm/src/sql/sql.ts +++ b/drizzle-orm/src/sql/sql.ts @@ -33,6 +33,7 @@ export interface BuildQueryConfig { escapeName(name: string): string; escapeParam(num: number, value: unknown): string; escapeString(str: string): string; + /** @internal */ codecs?: CodecsCollection; paramStartIndex?: { value: number }; inlineParams?: boolean; @@ -170,6 +171,7 @@ export class SQL implements SQLWrapper { } as Query; } + /** @internal */ private collectSQL( chunks: SQLChunk[], config: BuildQueryConfig, @@ -439,6 +441,7 @@ export class SQL implements SQLWrapper { } } + /** @internal */ private mapInlineParam( chunk: unknown, { escapeString }: BuildQueryConfig, @@ -520,7 +523,7 @@ export class SQL implements SQLWrapper { } } -export type GetDecoderResult = T extends Column ? T['_']['data'] : T extends +export type GetDecoderResult = T extends { _: { data: infer TData } } ? TData : T extends | DriverValueDecoder | DriverValueDecoder['mapFromDriverValue'] ? TData : never; diff --git a/drizzle-orm/tests/exports-resolution.test.ts b/drizzle-orm/tests/exports-resolution.test.ts index a0adf7a737..a394c320e1 100644 --- a/drizzle-orm/tests/exports-resolution.test.ts +++ b/drizzle-orm/tests/exports-resolution.test.ts @@ -3,6 +3,7 @@ import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync import { tmpdir } from 'node:os'; import { dirname, join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; +import ts from 'typescript'; import { beforeAll, describe, expect, test } from 'vitest'; import { checkPackage } from '../../attw-fork/src/checkPackage.ts'; import { getExitCode } from '../../attw-fork/src/cli/getExitCode.ts'; @@ -114,6 +115,67 @@ describe.skipIf(!ARTIFACTS_PRESENT)('no public subpath is dropped', () => { }, 120_000); }); +describe.skipIf(!ARTIFACTS_PRESENT)('types are portable across package instances', () => { + test('SQL types from separate installations are assignable', () => { + const outDir = mkdtempSync(join(tmpdir(), 'drizzle-duplicate-types-')); + try { + const sourcePrefix = `/node_modules/${pkg.packageName}/`; + for (const packageName of ['drizzle-a', 'drizzle-b']) { + for (const file of pkg.listFiles(sourcePrefix)) { + if (file !== `${sourcePrefix}package.json` && !file.endsWith('.d.ts')) continue; + + const destination = join(outDir, 'node_modules', packageName, file.slice(sourcePrefix.length)); + mkdirSync(dirname(destination), { recursive: true }); + const contents = pkg.readFile(file); + writeFileSync( + destination, + file === `${sourcePrefix}package.json` + ? JSON.stringify({ ...JSON.parse(contents), name: packageName }) + : contents, + ); + } + } + + const entrypoint = join(outDir, 'index.mts'); + writeFileSync( + entrypoint, + [ + "import type { SQL as SQLA } from 'drizzle-a/sql/sql';", + "import type { SQL as SQLB } from 'drizzle-b/sql/sql';", + 'declare const sqlA: SQLA;', + 'declare const sqlB: SQLB;', + 'const acceptsA: SQLA = sqlB;', + 'const acceptsB: SQLB = sqlA;', + 'void [acceptsA, acceptsB];', + ].join('\n'), + ); + + const program = ts.createProgram({ + rootNames: [entrypoint], + options: { + module: ts.ModuleKind.NodeNext, + moduleResolution: ts.ModuleResolutionKind.NodeNext, + noEmit: true, + skipLibCheck: true, + strict: true, + target: ts.ScriptTarget.ESNext, + }, + }); + const diagnostics = ts.getPreEmitDiagnostics(program); + expect( + diagnostics, + ts.formatDiagnosticsWithColorAndContext(diagnostics, { + getCanonicalFileName: (fileName) => fileName, + getCurrentDirectory: () => outDir, + getNewLine: () => '\n', + }), + ).toEqual([]); + } finally { + rmSync(outDir, { recursive: true, force: true }); + } + }); +}); + describe('directory-index shim emitter refuses to shadow a source artifact', () => { test('throws a path-named error on a synthetic collision', async () => { const outDir = mkdtempSync(join(tmpdir(), 'shim-guard-')); From b1d014a96441c97764c2b895d63fd6f3d632af73 Mon Sep 17 00:00:00 2001 From: Andrew Lee Date: Sat, 29 Aug 2026 12:07:26 -0600 Subject: [PATCH 2/2] Generalize the cross-instance type check into a portability matrix The single SQL assignment only guarded one type. Replace it with a table of public types probed in one tsc program, asserting both directions: a portable type regressing fails, and a known leak becoming portable fails too, prompting the flag to be flipped. Records the measured state of the surface, annotating each known leak with the private/protected member responsible. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Qd99Mmmi1fZkKQiV3u1jXy --- drizzle-orm/tests/exports-resolution.test.ts | 190 +++++++++++++++---- 1 file changed, 153 insertions(+), 37 deletions(-) diff --git a/drizzle-orm/tests/exports-resolution.test.ts b/drizzle-orm/tests/exports-resolution.test.ts index a394c320e1..b65e7ccab7 100644 --- a/drizzle-orm/tests/exports-resolution.test.ts +++ b/drizzle-orm/tests/exports-resolution.test.ts @@ -115,40 +115,132 @@ describe.skipIf(!ARTIFACTS_PRESENT)('no public subpath is dropped', () => { }, 120_000); }); +// Two physically distinct installs of the package produce two declaration sites for +// every class. TypeScript compares classes carrying `private`/`protected` members +// nominally, so any such member surviving into the emitted `.d.ts` makes that type +// non-portable: a value from copy A is not assignable to the same type from copy B. +// This is the failure a consumer hits whenever a transitive dep, a pnpm peer split +// or a linked tarball puts two copies of drizzle-orm on disk. +// +// The matrix below is the current, measured state of the public surface. `portable: +// false` entries are known leaks, not aspirations -- each is annotated with the member +// responsible. Fixing one is expected to flip its flag here; that is the point of +// asserting both directions. +interface PortabilityProbe { + /** Exported type name. */ + name: string; + /** Package subpath it is exported from. */ + subpath: string; + /** Type arguments, when the type has no usable defaults. */ + typeArgs?: string; + /** Whether a value of this type survives crossing between two installs. */ + portable: boolean; + /** For known leaks: the member whose nominality blocks assignment. */ + blockedBy?: string; +} + +const PORTABILITY_MATRIX: PortabilityProbe[] = [ + { name: 'SQL', subpath: 'sql/sql', portable: true }, + { name: 'Column', subpath: 'column', portable: true }, + { name: 'Table', subpath: 'table', portable: true }, + { name: 'Subquery', subpath: 'subquery', portable: true }, + { name: 'View', subpath: 'sql/sql', portable: true }, + { name: 'Placeholder', subpath: 'sql/sql', portable: true }, + { name: 'QueryPromise', subpath: 'query-promise', typeArgs: '', portable: true }, + { name: 'MySqlTable', subpath: 'mysql-core', portable: true }, + { name: 'MySqlColumn', subpath: 'mysql-core', portable: true }, + { name: 'SQLiteTable', subpath: 'sqlite-core', portable: true }, + { name: 'SQLiteColumn', subpath: 'sqlite-core', portable: true }, + + { name: 'Param', subpath: 'sql/sql', portable: false, blockedBy: 'Param#brand (protected)' }, + { name: 'Name', subpath: 'sql/sql', portable: false, blockedBy: 'Name#brand (protected)' }, + { + name: 'CodecsCollection', + subpath: 'codecs', + portable: false, + blockedBy: 'CodecsCollection#resolveTypes (protected)', + }, + // The three dialects all embed a CodecsCollection, so they inherit its leak. + { name: 'PgDialect', subpath: 'pg-core', portable: false, blockedBy: 'CodecsCollection#resolveTypes (protected)' }, + { + name: 'MySqlDialect', + subpath: 'mysql-core', + portable: false, + blockedBy: 'CodecsCollection#resolveTypes (protected)', + }, + { + name: 'SQLiteDialect', + subpath: 'sqlite-core', + portable: false, + blockedBy: 'CodecsCollection#resolveTypes (protected)', + }, + // Only pg-core exposes `toBuilder()` in a public return type, which is why the + // identical `foreignKeyConfigs` field on the other dialects' builders is unreachable + // and their table/column types above are portable. + { + name: 'PgTable', + subpath: 'pg-core', + portable: false, + blockedBy: 'PgColumnBuilder#foreignKeyConfigs (private), via PgColumn#toBuilder', + }, + { + name: 'PgColumn', + subpath: 'pg-core', + portable: false, + blockedBy: 'PgColumnBuilder#foreignKeyConfigs (private), via PgColumn#toBuilder', + }, + { + name: 'NodePgDatabase', + subpath: 'node-postgres/driver', + typeArgs: '>', + portable: false, + blockedBy: 'PgAsyncPreparedQuery#executor (protected)', + }, +]; + +// Unpacks the built tarball's declarations twice, under two different package names, +// so the compiler sees two independent installs of the same types. +function materializeTwoInstalls(outDir: string): void { + const sourcePrefix = `/node_modules/${pkg.packageName}/`; + for (const packageName of ['drizzle-a', 'drizzle-b']) { + for (const file of pkg.listFiles(sourcePrefix)) { + if (file !== `${sourcePrefix}package.json` && !file.endsWith('.d.ts')) continue; + + const destination = join(outDir, 'node_modules', packageName, file.slice(sourcePrefix.length)); + mkdirSync(dirname(destination), { recursive: true }); + const contents = pkg.readFile(file); + writeFileSync( + destination, + file === `${sourcePrefix}package.json` + ? JSON.stringify({ ...JSON.parse(contents), name: packageName }) + : contents, + ); + } + } +} + describe.skipIf(!ARTIFACTS_PRESENT)('types are portable across package instances', () => { - test('SQL types from separate installations are assignable', () => { + test('the published surface matches the recorded portability matrix', () => { const outDir = mkdtempSync(join(tmpdir(), 'drizzle-duplicate-types-')); try { - const sourcePrefix = `/node_modules/${pkg.packageName}/`; - for (const packageName of ['drizzle-a', 'drizzle-b']) { - for (const file of pkg.listFiles(sourcePrefix)) { - if (file !== `${sourcePrefix}package.json` && !file.endsWith('.d.ts')) continue; - - const destination = join(outDir, 'node_modules', packageName, file.slice(sourcePrefix.length)); - mkdirSync(dirname(destination), { recursive: true }); - const contents = pkg.readFile(file); - writeFileSync( - destination, - file === `${sourcePrefix}package.json` - ? JSON.stringify({ ...JSON.parse(contents), name: packageName }) - : contents, - ); - } + materializeTwoInstalls(outDir); + + // One program covering every probe: each assignment gets its own line so a + // diagnostic's line number identifies which type failed. + const lines: string[] = []; + const lineToName = new Map(); + for (const { name, subpath } of PORTABILITY_MATRIX) { + lines.push(`import type { ${name} as ${name}_A } from 'drizzle-a/${subpath}';`); + lines.push(`import type { ${name} as ${name}_B } from 'drizzle-b/${subpath}';`); + } + for (const { name, typeArgs = '' } of PORTABILITY_MATRIX) { + lines.push(`declare const v_${name}: ${name}_B${typeArgs};`); + lineToName.set(lines.length + 1, name); + lines.push(`export const c_${name}: ${name}_A${typeArgs} = v_${name};`); } const entrypoint = join(outDir, 'index.mts'); - writeFileSync( - entrypoint, - [ - "import type { SQL as SQLA } from 'drizzle-a/sql/sql';", - "import type { SQL as SQLB } from 'drizzle-b/sql/sql';", - 'declare const sqlA: SQLA;', - 'declare const sqlB: SQLB;', - 'const acceptsA: SQLA = sqlB;', - 'const acceptsB: SQLB = sqlA;', - 'void [acceptsA, acceptsB];', - ].join('\n'), - ); + writeFileSync(entrypoint, lines.join('\n') + '\n'); const program = ts.createProgram({ rootNames: [entrypoint], @@ -161,15 +253,39 @@ describe.skipIf(!ARTIFACTS_PRESENT)('types are portable across package instances target: ts.ScriptTarget.ESNext, }, }); - const diagnostics = ts.getPreEmitDiagnostics(program); - expect( - diagnostics, - ts.formatDiagnosticsWithColorAndContext(diagnostics, { - getCanonicalFileName: (fileName) => fileName, - getCurrentDirectory: () => outDir, - getNewLine: () => '\n', - }), - ).toEqual([]); + + const failed = new Map(); + const unattributed: string[] = []; + for (const diagnostic of ts.getPreEmitDiagnostics(program)) { + const message = ts.flattenDiagnosticMessageText(diagnostic.messageText, ' '); + if (!diagnostic.file || diagnostic.start === undefined) { + unattributed.push(message); + continue; + } + const line = diagnostic.file.getLineAndCharacterOfPosition(diagnostic.start).line + 1; + const name = lineToName.get(line); + // A diagnostic on a non-assignment line means the probe itself is malformed + // (bad subpath, wrong type arity) rather than a portability result. + if (name === undefined) { + unattributed.push(`line ${line}: ${message}`); + continue; + } + if (!failed.has(name)) failed.set(name, message); + } + + expect(unattributed, `malformed probes:\n${unattributed.join('\n')}`).toEqual([]); + + const actual = PORTABILITY_MATRIX.filter((p) => !failed.has(p.name)).map((p) => p.name).sort(); + const expected = PORTABILITY_MATRIX.filter((p) => p.portable).map((p) => p.name).sort(); + + const regressed = expected.filter((n) => !actual.includes(n)); + const fixed = actual.filter((n) => !expected.includes(n)); + const hint = [ + ...regressed.map((n) => `REGRESSED ${n} is no longer portable: ${failed.get(n)}`), + ...fixed.map((n) => `FIXED ${n} is now portable -- set portable: true in PORTABILITY_MATRIX`), + ].join('\n'); + + expect(actual, hint).toEqual(expected); } finally { rmSync(outDir, { recursive: true, force: true }); }