diff --git a/.changeset/driver-memory-update-upsert-honest-types.md b/.changeset/driver-memory-update-upsert-honest-types.md new file mode 100644 index 0000000000..c8066e5fd9 --- /dev/null +++ b/.changeset/driver-memory-update-upsert-honest-types.md @@ -0,0 +1,11 @@ +--- +'@objectstack/driver-memory': minor +--- + +feat(driver-memory): `update()` and `upsert()` publish their honest types (#13878) + +**BREAKING** for TypeScript consumers — a published TYPE-surface narrowing, the shape ADR-0087's 2026-08-30 addendum names (a published SDK method whose declared return moves off `any` onto the contract it always answered) — shipped as `minor` under the launch-window convention. The emitted `.d.ts` read `Promise` for both doors: the return types were inferred through the backing store's `any[]` rows, so the union collapsed and no caller was asked to narrow. They are now declared as the contract declares them — `update()` returns `Promise | null>` (the `null` arm is the non-`strictMode` miss the driver has always answered with), `upsert()` returns `Promise>`. A caller that read fields off `update()`'s result through the `any` now narrows the `null` arm first; a caller that leaned on `any` to read undeclared members of either result now types them. + +`upsert()` now asserts — throws on — the `null` arm of `update()` on a path it cannot reach (the row it updates was found in the same table a moment earlier, with nothing yielding in between), instead of widening its own declared return to carry an arm it can never produce. No reachable runtime behaviour changes. + + diff --git a/.changeset/idatadriver-update-declares-null.md b/.changeset/idatadriver-update-declares-null.md new file mode 100644 index 0000000000..9055dc47a5 --- /dev/null +++ b/.changeset/idatadriver-update-declares-null.md @@ -0,0 +1,11 @@ +--- +'@objectstack/spec': minor +--- + +feat(spec): `IDataDriver.update()` declares its not-found arm (#13878) + +**BREAKING** for TypeScript consumers, shipped as `minor` under the repo's launch-window convention for breaking changes (maintainer ruling 2026-09-01, option A). `IDataDriver.update()`'s return type is now `Promise | null>` — the shape `findOne()` already carries and the not-found vocabulary `delete()` already uses (`false`) — and its docblock says when `null` is returned: no record with that id exists, on a driver not configured to throw on missing records. The Zod mirror `DriverInterfaceSchema.update` carries the same `.nullable()` output and the same sentence. This declares the behaviour four of the six shipped drivers have always had, which the previous declaration forbade. + +What breaks: a caller that read fields straight off an `update()` result compiled before only because the arm was undeclared; it now narrows the `null` arm first — a compile-time obligation at the call site in place of a silent runtime hazard. No metadata key, no runtime behaviour and no wire shape moves. + + diff --git a/.changeset/metadata-database-loader-update-null.md b/.changeset/metadata-database-loader-update-null.md new file mode 100644 index 0000000000..e82f71e7e4 --- /dev/null +++ b/.changeset/metadata-database-loader-update-null.md @@ -0,0 +1,5 @@ +--- +'@objectstack/metadata': patch +--- + +`DatabaseLoader`'s private driver-path update helper is typed with the `null` arm `IDataDriver.update()` now declares; both of its callers discard the result. No runtime behaviour changes. diff --git a/packages/drivers/driver-memory/src/memory-driver.test.ts b/packages/drivers/driver-memory/src/memory-driver.test.ts index e29eaec97b..e189a1e000 100644 --- a/packages/drivers/driver-memory/src/memory-driver.test.ts +++ b/packages/drivers/driver-memory/src/memory-driver.test.ts @@ -67,7 +67,10 @@ describe('InMemoryDriver', () => { { active: false } ); - expect(updateResult.active).toBe(false); + // The declared type carries the not-found arm (#13878); a caller reading + // fields narrows first. The row exists, so the arm is not taken here. + expect(updateResult).not.toBeNull(); + expect(updateResult!.active).toBe(false); const results = await driver.find(testTable, { fields: ['active'] }); expect(results[0].active).toBe(false); @@ -98,8 +101,9 @@ describe('InMemoryDriver', () => { const originalCreatedAt = created.created_at; const updated = await driver.update(testTable, '1', { name: 'Alice Updated' }); - expect(updated.created_at).toBe(originalCreatedAt); - expect(updated.name).toBe('Alice Updated'); + expect(updated).not.toBeNull(); + expect(updated!.created_at).toBe(originalCreatedAt); + expect(updated!.name).toBe('Alice Updated'); }); }); diff --git a/packages/drivers/driver-memory/src/memory-driver.ts b/packages/drivers/driver-memory/src/memory-driver.ts index c5397c9ac9..bbc35279aa 100644 --- a/packages/drivers/driver-memory/src/memory-driver.ts +++ b/packages/drivers/driver-memory/src/memory-driver.ts @@ -666,7 +666,15 @@ export class InMemoryDriver implements IDataDriver { return { ...newRecord }; } - async update(object: string, id: string | number, data: Record, options?: DriverOptions) { + /** + * Declared as the contract declares it (#13878): the `null` arm is the + * non-`strictMode` miss the behaviour pin has always held, and the explicit + * annotation is what keeps that arm visible to `tsc` — left to inference, + * the return type collapses to `any` through the backing store's + * `any[]` rows and the union is swallowed, so the published `.d.ts` read + * `Promise` and no caller was ever asked to narrow. + */ + async update(object: string, id: string | number, data: Record, options?: DriverOptions): Promise | null> { this.logger.debug('Update operation', { object, id }); const table = this.getTable(object); @@ -698,7 +706,7 @@ export class InMemoryDriver implements IDataDriver { return { ...updatedRecord }; } - async upsert(object: string, data: Record, conflictKeys?: string[], options?: DriverOptions) { + async upsert(object: string, data: Record, conflictKeys?: string[], options?: DriverOptions): Promise> { this.logger.debug('Upsert operation', { object, conflictKeys }); const table = this.getTable(object); @@ -712,7 +720,16 @@ export class InMemoryDriver implements IDataDriver { if (existingRecord) { this.logger.debug('Record exists, updating', { object, id: existingRecord.id }); - return this.update(object, existingRecord.id, data, options); + const updated = await this.update(object, existingRecord.id, data, options); + // `existingRecord` was read from this same table above and nothing + // yields in between, so `update` cannot miss here: the `null` arm of + // its declared type is unreachable on this path. An upsert never + // answers "not found" — say so loudly rather than widen this door's + // declared return to carry an arm it can never produce (#13878). + if (updated === null) { + throw new Error(`Record with ID ${existingRecord.id} vanished from ${object} during upsert`); + } + return updated; } else { this.logger.debug('Record does not exist, creating', { object }); return this.create(object, data, options); diff --git a/packages/drivers/driver-memory/src/memory-update-declared-null.test.ts b/packages/drivers/driver-memory/src/memory-update-declared-null.test.ts new file mode 100644 index 0000000000..78e59088d8 --- /dev/null +++ b/packages/drivers/driver-memory/src/memory-update-declared-null.test.ts @@ -0,0 +1,88 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// #13878 — the declared return type of `update()` carries its not-found arm, +// and the driver's published type is the contract's, not `any`. +// +// `InMemoryDriver.update()` has always answered a missing id with `null` when +// `strictMode` is off (the behaviour pin in `memory-driver.test.ts` holds it), +// while `IDataDriver.update()` declared `Promise>` — +// a contract violated by 4 of 6 shipped drivers and never seen by `tsc`, +// because the driver's return type was INFERRED through the backing store's +// `any[]` rows: the union collapsed to `any`, the published `.d.ts` read +// `Promise`, and no caller was ever asked to narrow. The maintainer's +// ruling (2026-09-01) declares the arm — `Promise | +// null>`, the shape `findOne` already carries — and un-masks the driver. +// +// This file pins BOTH halves at the type level, inside the package's tsc +// program (`tsconfig.json` selects `src/**/*`, tests included): +// +// 1. the CONTRACT: `IDataDriver.update()` resolves to +// `Record | null` — read through `@objectstack/spec`'s +// built `.d.ts`, so reverting the declaration alone reds this file; +// 2. the DRIVER: `InMemoryDriver.update()` / `upsert()` are not `any` and +// resolve to exactly the contract's types — reverting the driver's +// explicit annotations alone reds this file too (the `any` mask +// returns and `IsAny` flips). +// +// The `satisfies`-free typed-const form is the one the sibling drivers use +// (`turso-driver-options-door.test.ts`, `sql-driver-distinct-filter- +// narrowing.test.ts`); the runtime cases below make the consts observable so +// the file is a test and not a declaration. + +import { describe, it, expect } from 'vitest'; +import { InMemoryDriver } from './memory-driver.js'; +import type { IDataDriver } from '@objectstack/spec/contracts'; + +/** `any` defeats ordinary assignability checks; this is the standard detector. */ +type IsAny = 0 extends 1 & T ? true : false; +/** Exact (mutual, non-`any`) type equality. */ +type Equals = (() => T extends A ? 1 : 2) extends (() => T extends B ? 1 : 2) ? true : false; + +type ContractUpdate = Awaited>; +type MemoryUpdate = Awaited>; +type MemoryUpsert = Awaited>; + +// 1. The contract declares the not-found arm. +const contractUpdateDeclaresNull: Equals | null> = true; + +// 2. The driver's doors are un-masked and read exactly as the contract does. +const memoryUpdateIsAny: IsAny = false; +const memoryUpdateIsContract: Equals | null> = true; +const memoryUpsertIsAny: IsAny = false; +const memoryUpsertIsContract: Equals> = true; + +describe('InMemoryDriver.update()/upsert() declared return types (#13878)', () => { + it('pins the contract and the driver at the type level', () => { + expect([ + contractUpdateDeclaresNull, + memoryUpdateIsAny, + memoryUpdateIsContract, + memoryUpsertIsAny, + memoryUpsertIsContract, + ]).toEqual([true, false, true, false, true]); + }); + + it('update() on a missing id resolves to null, and the declared type makes the caller narrow', async () => { + const driver = new InMemoryDriver(); + await driver.connect(); + + const result = await driver.update('t', 'missing', { name: 'x' }); + expect(result).toBeNull(); + + // The narrowing the declared type now demands of every caller: a field + // read is only reachable behind the `null` check. + const name = result === null ? 'absent' : result.name; + expect(name).toBe('absent'); + }); + + it('upsert() over an existing id returns the merged record and never the not-found arm', async () => { + const driver = new InMemoryDriver(); + await driver.connect(); + await driver.create('t', { id: '1', name: 'before', keep: 'kept' }); + + const merged = await driver.upsert('t', { id: '1', name: 'after' }); + expect(merged.id).toBe('1'); + expect(merged.name).toBe('after'); + expect(merged.keep).toBe('kept'); + }); +}); diff --git a/packages/metadata/src/loaders/database-loader.ts b/packages/metadata/src/loaders/database-loader.ts index 9e1f2aa097..dcc400b85f 100644 --- a/packages/metadata/src/loaders/database-loader.ts +++ b/packages/metadata/src/loaders/database-loader.ts @@ -307,7 +307,9 @@ export class DatabaseLoader implements MetadataLoader { return this.driver!.create(table, data); } - private async _update(table: string, id: string, data: Record): Promise> { + // `null` is the driver path's not-found answer (`IDataDriver.update()`, + // #13878); both callers here resolve the row first and discard the result. + private async _update(table: string, id: string, data: Record): Promise | null> { if (this.engine) { // [#11231] The resolved id AFTER the spread, so it WINS — the same // convention as `rest-server.ts`'s batch arm and `protocol.updateData` diff --git a/packages/spec/src/contracts/data-driver.ts b/packages/spec/src/contracts/data-driver.ts index fd63a5a2d7..8bec163178 100644 --- a/packages/spec/src/contracts/data-driver.ts +++ b/packages/spec/src/contracts/data-driver.ts @@ -189,8 +189,13 @@ export interface IDataDriver { /** * Update an existing record by ID. * MUST return `id` as string. MUST NOT return implementation details like `_id`. + * @returns The updated record, or `null` if no record with that id exists — + * the same "addressed one row, which may not exist" shape as {@link findOne}, + * and the not-found vocabulary {@link delete} already carries (`false`). A + * driver configured to throw on missing records (`strictMode`) throws + * instead of returning `null`. */ - update(object: string, id: string | number, data: Record, options?: DriverOptions): Promise>; + update(object: string, id: string | number, data: Record, options?: DriverOptions): Promise | null>; /** * Upsert (Update or Insert) a record. diff --git a/packages/spec/src/data/driver.zod.ts b/packages/spec/src/data/driver.zod.ts index 51b6eee585..1e452f531b 100644 --- a/packages/spec/src/data/driver.zod.ts +++ b/packages/spec/src/data/driver.zod.ts @@ -504,12 +504,13 @@ export const DriverInterfaceSchema = lazySchema(() => z.object({ * @param id - The unique identifier of the record. * @param data - The fields to update. * @param options - Driver options. - * @returns The updated record. + * @returns The updated record, or `null` if no record with that id exists + * (a driver configured to throw on missing records throws instead). * MUST return `id` as string. MUST NOT return implementation details like `_id`. */ update: z.function() .input(z.tuple([z.string(), z.string().or(z.number()), z.record(z.string(), z.unknown()), DriverOptionsSchema.optional()])) - .output(z.promise(z.record(z.string(), z.unknown()))) + .output(z.promise(z.record(z.string(), z.unknown()).nullable())) .describe('Update record'), /**