also looking at this
fix(client): keep client.d.ts free of `any`, and make the guard that says so run
#1132
3 files
+62
-8
| @@ -108,6 +108,12 @@ jobs: | ||
| 108 | 108 | echo "Waiting for PostgreSQL to be ready..." |
| 109 | 109 | timeout 60 bash -c 'until pg_isready -h localhost -p 5432 -U postgres; do sleep 1; done' |
| 110 | 110 | echo "PostgreSQL is ready!" |
| 111 | # public-surface-types.test.ts reads dist/*.d.ts. Without this it found no | |
| 112 | # dist/, returned early, and checked nothing on every CI run — which is | |
| 113 | # how an `any` reached client.d.ts after #1117 took that file to zero. | |
| 114 | - name: Build | |
| 115 | run: bun run build | |
| 116 | ||
| 111 | 117 | - name: Unit Tests |
| 112 | 118 | run: bun run test |
| 113 | 119 | |
| @@ -640,8 +640,14 @@ export interface PaginateOptions { | ||
| 640 | 640 | perPage?: number |
| 641 | 641 | /** 1-based page number. Defaults to 1. */ |
| 642 | 642 | page?: number |
| 643 | /** Run the count and the page query inside this transaction — see #1051. */ | |
| 644 | tx?: { unsafe: (sql: string, params?: any[]) => any } | |
| 643 | /** | |
| 644 | * Run the count and the page query inside this transaction — see #1051. | |
| 645 | * | |
| 646 | * `unknown` rather than `any`, matching the positional overload's declared | |
| 647 | * `opts.tx`. An `any` here is published in `client.d.ts`, which is the type | |
| 648 | * an app touches on every query — #1117 took that file to zero and pinned it. | |
| 649 | */ | |
| 650 | tx?: { unsafe: (sql: string, params?: unknown[]) => unknown } | |
| 645 | 651 | } |
| 646 | 652 | |
| 647 | 653 | export type QueryResult = unknown |
| @@ -5854,7 +5860,7 @@ export function createQueryBuilder<DB extends AnyDatabaseSchema>(state?: Partial | ||
| 5854 | 5860 | if (typeof perPage === 'object' && perPage !== null) { |
| 5855 | 5861 | const options = perPage |
| 5856 | 5862 | page = options.page ?? 1 |
| 5857 | opts = options.tx ? { tx: options.tx } : {} | |
| 5863 | opts = options.tx ? { tx: options.tx as { unsafe: (sql: string, params?: any[]) => any } } : {} | |
| 5858 | 5864 | perPage = options.perPage ?? 10 |
| 5859 | 5865 | } |
| 5860 | 5866 | if (!Number.isFinite(perPage) || perPage <= 0 || !Number.isInteger(perPage)) |
| @@ -1,5 +1,5 @@ | ||
| 1 | 1 | import { describe, expect, it } from 'bun:test' |
| 2 | import { existsSync, readdirSync, readFileSync } from 'node:fs' | |
| 2 | import { existsSync, readdirSync, readFileSync, statSync } from 'node:fs' | |
| 3 | 3 | import { join } from 'node:path' |
| 4 | 4 | |
| 5 | 5 | /** |
| @@ -29,18 +29,60 @@ import { join } from 'node:path' | ||
| 29 | 29 | */ |
| 30 | 30 | |
| 31 | 31 | const DIST = join(import.meta.dir, '..', 'dist') |
| 32 | const SRC = join(import.meta.dir, '..', 'src') | |
| 32 | 33 | const BASELINE = 18 |
| 33 | 34 | |
| 34 | 35 | function anyCount(source: string): number { |
| 35 | 36 | return (source.match(/:\s*any\b|<any>|any\[\]/g) ?? []).length |
| 36 | 37 | } |
| 37 | 38 | |
| 39 | /** | |
| 40 | * Whether `dist/` reflects the `src/` in this working tree. | |
| 41 | * | |
| 42 | * This guard reads a build output, so it has two failure modes that are not | |
| 43 | * regressions and must not be reported as ones: | |
| 44 | * | |
| 45 | * - `dist/` missing entirely — nobody has built yet | |
| 46 | * - `dist/` older than `src/` — built before the change under test, so the | |
| 47 | * counts describe code that is no longer here. That produced a red suite | |
| 48 | * twice for people whose checkout predated #1117, on a defect he had | |
| 49 | * already fixed. | |
| 50 | * | |
| 51 | * In CI the same conditions mean the opposite: the workflow builds before | |
| 52 | * testing, so a missing or stale `dist/` means the build step did not run, and | |
| 53 | * skipping would be how a real leak ships unnoticed. That is exactly what | |
| 54 | * happened — the CI `test` job never built, so this returned early on every | |
| 55 | * run and never checked anything until the build step was added alongside | |
| 56 | * this comment. | |
| 57 | */ | |
| 58 | function distState(): 'fresh' | 'missing' | 'stale' { | |
| 59 | if (!existsSync(DIST)) | |
| 60 | return 'missing' | |
| 61 | const declarations = readdirSync(DIST).filter(file => file.endsWith('.d.ts')) | |
| 62 | if (declarations.length === 0) | |
| 63 | return 'missing' | |
| 64 | const builtAt = Math.min(...declarations.map(f => statSync(join(DIST, f)).mtimeMs)) | |
| 65 | const changedAt = Math.max(...readdirSync(SRC).map(f => statSync(join(SRC, f)).mtimeMs)) | |
| 66 | return changedAt > builtAt ? 'stale' : 'fresh' | |
| 67 | } | |
| 68 | ||
| 69 | const STATE = distState() | |
| 70 | const CI = Boolean(process.env.CI) | |
| 71 | ||
| 72 | function assertUsable(): boolean { | |
| 73 | if (STATE === 'fresh') | |
| 74 | return true | |
| 75 | if (CI) | |
| 76 | throw new Error(`[public-surface] dist/ is ${STATE} in CI — the build step must run before these tests, or this guard silently checks nothing.`) | |
| 77 | // Locally: say why it did not run, rather than asserting against stale output. | |
| 78 | console.warn(`[public-surface] skipped: dist/ is ${STATE}. Run \`bun run build\` to check the published types.`) | |
| 79 | return false | |
| 80 | } | |
| 81 | ||
| 38 | 82 | describe('the published type surface', () => { |
| 39 | 83 | it('does not leak more `any` than the documented survivors', () => { |
| 40 | if (!existsSync(DIST)) { | |
| 41 | // Types are a build output; nothing to check before `bun run build`. | |
| 84 | if (!assertUsable()) | |
| 42 | 85 | return |
| 43 | } | |
| 44 | 86 | |
| 45 | 87 | const declarations = readdirSync(DIST).filter(file => file.endsWith('.d.ts')) |
| 46 | 88 | expect(declarations.length).toBeGreaterThan(0) |
| @@ -54,7 +96,7 @@ describe('the published type surface', () => { | ||
| 54 | 96 | }) |
| 55 | 97 | |
| 56 | 98 | it('keeps the query builder itself free of them', () => { |
| 57 | if (!existsSync(join(DIST, 'client.d.ts'))) | |
| 99 | if (!assertUsable()) | |
| 58 | 100 | return |
| 59 | 101 | |
| 60 | 102 | // client.d.ts is the type an app touches on every single query. It went |