diff --git a/.changeset/sql-driver-init-objects-indexes-param.md b/.changeset/sql-driver-init-objects-indexes-param.md new file mode 100644 index 0000000000..79c3dd097f --- /dev/null +++ b/.changeset/sql-driver-init-objects-indexes-param.md @@ -0,0 +1,34 @@ +--- +"@objectstack/driver-sql": minor +"@objectstack/driver-sqlite-wasm": minor +--- + +`SqlDriver.initObjects()` and `SqlDriver.registerObjectMetadata()` now declare the `indexes` key they have always read. + +Both entry points took `Array<{ name; fields?; tenancy? }>`, with no `indexes` in the type. The key was read out of those very objects one call deep anyway, through an `as any`, in `registerManagedObjectMetadata` — and the map it fills, `managedObjectIndexes`, is what `syncDeclaredIndexes` renders every declared UNIQUE from. So the driver's whole index-sync path was driven by a key its own signature said did not exist, while the sibling `detectManagedDrift` on the same class had always declared `indexes?: any[]`: the two halves of one class disagreed about the shape of the same input. + +That is the shape #4311 already addressed for `tenancy`, one key over, and the comment it left above `initObjects` described `indexes` word for word. + +**Why nothing tripped over it.** TypeScript's excess-property check fires on a fresh object literal and not on one bound to a variable first, so the same object was accepted or rejected by nothing but where it was spelled — `await driver.initObjects([{ ...bare, indexes: [] }])` was rejected with TS2353, `const o = { ...bare, indexes: [] }; await driver.initObjects([o])` was accepted, and the index was synced either way. Every caller happened to bind first, so the package typechecked green for a reason unrelated to correctness. + +**Why this matters beyond a compile error.** The loud symptom was a rejected correct call. The quiet one is the reachable branch: an author — or an AI — reading the signature concludes `indexes` is not accepted and drops the key, and a declared UNIQUE is then never synced, with no error at authoring time and no error at boot. The schema says those rows cannot collide; they can. + +What changed, all inside `SqlDriver`: + +- `registerObjectMetadata(objects)`, `initObjects(objects)` and the shared `registerManagedObjectMetadata(obj)` helper each gained `indexes?: any[]`, spelled exactly as `detectManagedDrift` already spells it. +- Every `(obj as any)` cast reading `indexes` off those parameters is gone — the one at the `managedObjectIndexes.set` site and the two inside `initObjects`' own create/alter path. The cast was the evidence that the declaration and the read disagreed; leaving any of them would have fixed the signature while keeping the "the type does not admit me but I read it anyway" path alive. That path is now closed on this parameter. + +**What the accept set does, precisely — it moves in both directions.** For a **fresh object literal**, which is what an author writes and what the excess-property check judges, this is purely a widening: `{ ...bare, indexes: [...] }` was rejected and is now accepted. For a **variable-bound** argument, which bypasses that check and is judged by ordinary assignability, it is a narrowing: `indexes` spelled as a record, as a `readonly` tuple (`as const`), or as `null` compiled under the previous signatures and is now rejected with TS2322. Measured in both directions, all three shapes, on this package's own `tsc`. + +That narrowing is deliberate, and the three shapes did **not** all behave the same way before it — the difference is worth stating exactly, because only one of them ever worked: + +- A **record** and **`null`** never survived the `Array.isArray(obj.indexes)` guard the driver has always applied. That author got no index and no diagnostic — silently, at run time. Rejecting those two at compile time is precisely the failure this change exists to make impossible. +- A **`readonly` tuple (`as const`)** is a different case, and the only one with anything to lose. `as const` is type-only: at run time the value is a plain array, `Array.isArray` returns `true`, and the index **was** synced. That caller compiled and worked, and is now rejected at compile time. Nothing about its run-time behaviour changed — the rejection is entirely on the type surface. + +No migration is owed even so. No caller in this repository is affected, and the shape could never have reached `detectManagedDrift` on the same class either, which publishes the very same `any[]` spelling for the very same key — so a `readonly` caller was already unable to use half of this driver's declared-index surface. A caller in that position spells the array without `as const`, or widens it at the call site. + +The disposition on that corrected ground, recorded here because the ground itself moved: **no `BREAKING` banner and no ADR-0087 disposition**, resting on grounds (i) and (iii) alone — zero affected callers, and the `any[]` spelling already published on `detectManagedDrift` for the same key on the same class. The ground that every newly rejected shape had already been discarded at run time is **not** among them: it is false for the `readonly` tuple, and nothing here leans on it. + +`@objectstack/driver-sqlite-wasm` is named because `SqliteWasmDriver extends SqlDriver` and overrides neither method, so both widened signatures land in its own published `.d.ts` and its consumers see the identical change. The two packages are in the same fixed version group, so this is a CHANGELOG effect rather than a version one. + +The `IDataDriver` contract itself did not move: `registerObjectMetadata?(schemas: unknown[])` in `@objectstack/spec` already accepted `unknown[]`, and `SqlDriver` narrowed it on its own. What grew is `SqlDriver`'s own published accept set. diff --git a/packages/drivers/driver-sql/src/sql-driver-16570-init-objects-indexes-param.test.ts b/packages/drivers/driver-sql/src/sql-driver-16570-init-objects-indexes-param.test.ts new file mode 100644 index 0000000000..e19b3c6dbb --- /dev/null +++ b/packages/drivers/driver-sql/src/sql-driver-16570-init-objects-indexes-param.test.ts @@ -0,0 +1,149 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#16570] `initObjects` and `registerObjectMetadata` accept `indexes` — the + * key they have always READ — spelled as a **fresh object literal**. + * + * ## The defect this pins + * + * Both entry points declared `Array<{ name; fields?; tenancy? }>`, with no + * `indexes`. The key was read one call deep anyway, through an `as any`, in + * `registerManagedObjectMetadata`: + * + * ```ts + * this.managedObjectIndexes.set(tableName, (obj as any).indexes); + * ``` + * + * and `managedObjectIndexes` is what `syncDeclaredIndexes` renders every + * declared UNIQUE from — so the driver's whole index-sync path was driven by a + * key its own signature said did not exist. `detectManagedDrift`, on the same + * class, had always declared `indexes?: any[]`: the two halves of one class + * disagreed about the shape of the same input. That is the shape #4311 already + * fixed for `tenancy`, and the comment it left above `initObjects` described + * `indexes` word for word. + * + * ## Why the FORM of this pin is the whole point + * + * TypeScript's excess-property check fires on a **fresh object literal** and + * not on one bound to a variable first, so the same object was accepted or + * rejected by where it was spelled: + * + * ```ts + * await driver.initObjects([{ ...bare, indexes: [] }]); // TS2353 + * const withoutIndex = { ...bare, indexes: [] }; + * await driver.initObjects([withoutIndex]); // accepted + * ``` + * + * Every existing caller in this package happened to bind first — one of them + * (`sql-driver-11794-richtext-text-family.test.ts`) even wrote the workaround + * down: *"Hoisted (not an inline literal) … `indexes` rides through + * `initObjects` beyond its narrow parameter type"*. So the package typechecked + * green for a reason unrelated to correctness, and a **variable-bound pin + * cannot go red on this defect** — it measures nothing. Every call below is + * therefore an inline literal in argument position, which is what makes + * `tsc --noEmit` (this package's `typecheck` script) the instrument that + * measures it: revert either signature and these lines stop compiling with + * + * TS2353: Object literal may only specify known properties, and 'indexes' + * does not exist in type '{ name: string; fields?: Record + * | undefined; tenancy?: any; }'. + * + * The runtime assertions are the other half: they prove the key is not merely + * *admitted* by the type but still *read* — recorded in `managedObjectIndexes` + * (§1), rendered into a physical UNIQUE (§2), and cleared when withdrawn (§3). + * A signature relaxation that quietly stopped reading the key would pass the + * compile leg alone. + * + * Runs on the always-available in-memory SQLite cell: the defect is in a + * parameter type and in the registry it feeds, neither of which is + * dialect-specific. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { SqlDriver } from './sql-driver.js'; +import { dialectCell } from './live-dialect-matrix.testkit.js'; + +const SQLITE = dialectCell('sqlite'); + +/** + * The un-indexed base object, deliberately WITHOUT `indexes` — every call site + * below spreads it and writes `indexes` inline, so the literal being checked is + * fresh in argument position. `tenancy: { enabled: false }` keeps the declared + * UNIQUE on the plain (non-tenant-scoped) path, and the bounded `maxLength` + * keeps `v` a keyable varchar rather than an unbounded TEXT. + */ +const bareObject = (name: string) => ({ + name, + tenancy: { enabled: false }, + fields: { v: { type: 'text', maxLength: 64 } }, +}); + +/** What the driver recorded for `table`, read off the protected registry. */ +const recordedIndexes = (driver: SqlDriver, table: string): unknown => + (driver as unknown as { managedObjectIndexes: Map }).managedObjectIndexes.get(table); + +describe('initObjects / registerObjectMetadata accept `indexes` as a fresh object literal (#16570)', () => { + let driver: SqlDriver | undefined; + afterEach(async () => { + await driver?.disconnect().catch(() => {}); + driver = undefined; + }); + + it('§1 registerObjectMetadata: the inline literal compiles AND the key is recorded', async () => { + const T = 'os16570_register'; + driver = new SqlDriver(SQLITE.config()); + const declared = [{ fields: ['v'], unique: true as const, name: `uniq_${T}_v` }]; + + // Fresh literal in argument position — not hoisted to a variable first. + driver.registerObjectMetadata([{ ...bareObject(T), indexes: declared }]); + + expect(recordedIndexes(driver, T)).toEqual(declared); + }); + + it('§2 initObjects: the inline literal compiles AND the declared UNIQUE is physically synced', async () => { + const T = 'os16570_init'; + driver = new SqlDriver(SQLITE.config()); + + // Fresh literal in argument position. + await driver.initObjects([ + { ...bareObject(T), indexes: [{ fields: ['v'], unique: true as const, name: `uniq_${T}_v` }] }, + ]); + + const knex = (driver as unknown as { knex: (t: string) => any }).knex; + await knex(T).insert({ id: 'a', v: 'same' }); + // If `indexes` had been dropped at authoring time — the silent failure mode + // this card is about — this second row would be accepted. + await expect(knex(T).insert({ id: 'b', v: 'same' })).rejects.toThrow(); + }); + + it('§3 initObjects: the exact `{ ...bare, indexes: [] }` spelling from the card compiles, and withdraws the entry', async () => { + const T = 'os16570_withdraw'; + driver = new SqlDriver(SQLITE.config()); + const bare = bareObject(T); + + await driver.initObjects([{ ...bare, indexes: [{ fields: ['v'], unique: true as const, name: `uniq_${T}_v` }] }]); + expect(recordedIndexes(driver, T)).toHaveLength(1); + + // The spelling named in the card, verbatim: an empty array must CLEAR the + // entry, not leave the previous one standing. + await driver.initObjects([{ ...bare, indexes: [] }]); + expect(recordedIndexes(driver, T)).toEqual([]); + }); +}); + +/** + * The other half of the accept set. A variable-bound argument bypasses the + * excess-property check and is judged by ordinary assignability, so `indexes` + * must be an array — the two `@ts-expect-error`s ARE the assertion here: if + * either line stops erroring, `tsc` fails it as TS2578. Compile-time only, + * deliberately never called. + */ +export async function pinsTheNarrowingAxis(driver: SqlDriver): Promise { + const asRecord = { ...bareObject('os16570_narrow'), indexes: { uniq_v: { fields: ['v'] } } }; + // @ts-expect-error TS2322 — a record is not `any[]`, and never synced an index at run time. + await driver.initObjects([asRecord]); + + const asNull = { ...bareObject('os16570_narrow'), indexes: null }; + // @ts-expect-error TS2322 — `null` is not `any[]`, and never synced an index at run time. + await driver.initObjects([asNull]); +} diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index 4a0a0ed951..a142d4aa4b 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -9670,7 +9670,7 @@ export class SqlDriver implements IDataDriver { * which `initObjects` goes on to use for its DDL. */ protected registerManagedObjectMetadata( - obj: { name: string; fields?: Record; tenancy?: any }, + obj: { name: string; fields?: Record; tenancy?: any; indexes?: any[] }, ): { tableName: string; tenantField: string | null } { const tableName = StorageNameMapping.resolveTableName(obj); // #2186: remember the authoritative metadata field set for this table so @@ -9679,8 +9679,8 @@ export class SqlDriver implements IDataDriver { // Always overwrite — a metadata change that REMOVES `indexes` must clear // the previous entry, or drift detection keeps expecting an index nobody // declares any more (and never reports it as orphaned). - if (Array.isArray((obj as any).indexes)) { - this.managedObjectIndexes.set(tableName, (obj as any).indexes); + if (Array.isArray(obj.indexes)) { + this.managedObjectIndexes.set(tableName, obj.indexes); } else { this.managedObjectIndexes.delete(tableName); } @@ -9800,7 +9800,7 @@ export class SqlDriver implements IDataDriver { * Idempotent: pure metadata assignment, safe to re-drive on every reload. */ registerObjectMetadata( - objects: Array<{ name: string; fields?: Record; tenancy?: any }>, + objects: Array<{ name: string; fields?: Record; tenancy?: any; indexes?: any[] }>, ): void { for (const obj of objects) this.registerManagedObjectMetadata(obj); } @@ -9811,7 +9811,22 @@ export class SqlDriver implements IDataDriver { // undeclared here until #4311 (`registerExternalObject` and // `computeAndRecordTenantField` both had it), so a caller spelling the key // correctly was rejected by the type while the driver read it regardless. - async initObjects(objects: Array<{ name: string; fields?: Record; tenancy?: any }>): Promise { + // + // `indexes` is the same story, one key over, and it went undeclared here for + // longer: `registerManagedObjectMetadata` fills `managedObjectIndexes` from + // it, and that map is what `syncDeclaredIndexes` renders every declared + // UNIQUE from — so the whole index-sync path was driven by a key this + // signature said did not exist, reached through an `as any`. The sibling + // `detectManagedDrift` on this class had always declared it, so the two + // halves disagreed about the shape of the same input. Nothing tripped over + // it because TypeScript's excess-property check fires on a FRESH object + // literal and not on one bound to a variable first, and every caller here + // happened to bind first — a green that held for a reason unrelated to + // correctness. `src/sql-driver-16570-init-objects-indexes-param.test.ts` + // pins the fresh-literal form so it cannot silently go back. + async initObjects( + objects: Array<{ name: string; fields?: Record; tenancy?: any; indexes?: any[] }>, + ): Promise { // In-memory registration FIRST, and deliberately ahead of the DDL gate // below: being refused permission to alter a schema is not a reason to stay // ignorant of the objects we were just told about. On a datasource we are a @@ -9909,7 +9924,7 @@ export class SqlDriver implements IDataDriver { table: tableName, fields: obj.fields ?? {}, tenantField, - declaredIndexes: (obj as any).indexes, + declaredIndexes: obj.indexes, }); if (!exists) { @@ -9984,7 +9999,7 @@ export class SqlDriver implements IDataDriver { // referenced column physically exists — which is also why field-level // `unique` can no longer be emitted inline by `createColumn`: a composite // needs the tenant column to already be there. - const declaredIndexes = (obj as any).indexes; + const declaredIndexes = obj.indexes; const uniqueFields = Object.values(obj.fields ?? {}).some((f) => isUniqueScopeDeclared(f?.unique), );