From 82aa98e1c8c28e9fe3180afe3c06a15afc238aa7 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 13 Aug 2026 10:39:59 +0000 Subject: [PATCH] fix(types): the DataSource contract carries deleteView's per-home outcomes (#4564) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #4562 (#4479) widened the ObjectStack adapter's deleteView to return DeleteViewResult { deleted, draft?, published? }, but the shared interface still declared the narrow Promise of { deleted: boolean }. Nothing failed to compile — a wider return is assignable to a narrower declaration — so the adapter satisfied the interface while every consumer reaching it THROUGH DataSource was handed a type with the per-home outcomes already discarded. DeleteViewResult and ViewHomeDeleteOutcome move to packages/types/src/data.ts beside the interface that returns them, and deleteView?'s declared return widens to Promise of DeleteViewResult (optionality and parameters unchanged). data-objectstack imports them for its own use and re-exports both names, so every existing importer keeps compiling and now resolves to the same declaration the shared contract speaks. Census before the move: zero importers of either name outside the declaring file, PR #4562's own suite included. Pinned by deleteViewContract.types.test.ts. The pins are compile-time because the defect is: vitest erases types, so the consumer read below was green against the narrow declaration too — measured, 7/7 green with the fix reverted, while tsc returned 14 diagnostics. A live NarrowLegacyResult control keeps the discrimination honest. Note that `satisfies` and `extends` assertions are green in BOTH worlds for the same assignability reason the gap exploited; only the type IDENTITY assertions discriminate. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3 --- ...eteview-contract-per-home-outcomes-4564.md | 49 ++++ .../src/deleteViewContract.types.test.ts | 246 ++++++++++++++++++ packages/data-objectstack/src/index.ts | 57 ++-- packages/types/src/data.ts | 58 ++++- packages/types/src/index.ts | 2 + 5 files changed, 377 insertions(+), 35 deletions(-) create mode 100644 .changeset/deleteview-contract-per-home-outcomes-4564.md create mode 100644 packages/data-objectstack/src/deleteViewContract.types.test.ts diff --git a/.changeset/deleteview-contract-per-home-outcomes-4564.md b/.changeset/deleteview-contract-per-home-outcomes-4564.md new file mode 100644 index 0000000000..3dd2db8a29 --- /dev/null +++ b/.changeset/deleteview-contract-per-home-outcomes-4564.md @@ -0,0 +1,49 @@ +--- +'@object-ui/types': minor +'@object-ui/data-objectstack': minor +--- + +The `DataSource` contract carries `deleteView`'s per-home outcomes (#4564) + +#4479 / PR #4562 widened the ObjectStack adapter's `deleteView` to return +`DeleteViewResult { deleted, draft?, published? }`, so a caller could finally tell a +partial delete ("draft gone, published overlay left") from a complete one. The shared +interface did not follow: `DataSource.deleteView?` still declared the narrow +`Promise<{ deleted: boolean }>`. + +Nothing failed to compile, and that is exactly what made the gap invisible — a wider +return is assignable to a narrower declaration, so the adapter satisfied the interface +while every consumer reaching it **through** `DataSource` was handed a type with the +per-home outcomes already discarded. The one real call site today (app-shell's +`ObjectView` delete handler) awaits the call and reads nothing off the receipt, so the +loss was latent rather than broken. + +`DeleteViewResult` and `ViewHomeDeleteOutcome` now live in `@object-ui/types`, beside +the `DataSource` interface that returns them, and `deleteView?`'s declared return is +`Promise`. The direction was forced: the dependency runs +`@object-ui/data-objectstack` to `@object-ui/types` and never the other way, so the +shapes could not be imported downward — moving them was the alternative to re-declaring +a structural twin in `types`, which the one-resolver rule rejects because a copy is +mutually assignable with the original for exactly as long as it takes to drift. + +`@object-ui/data-objectstack` re-exports both names unchanged, so every importer PR +#4562 left pointing at it keeps compiling — and now resolves to the same declaration the +shared contract speaks rather than a look-alike. A repo-wide census before the move +found zero importers of either name outside the declaring file itself, PR #4562's own +suite included, so the re-export is insurance rather than a load-bearing shim. + +`deleteView` stays **optional** on the interface and keeps both parameters; the growth is +to the return type only, and `deleted` is untouched, so a consumer reading only `deleted` +needs no edit. + +Grading, per this repository's version-alignment convention (the major tracks +`@objectstack`, never an API-break count): + +- `@object-ui/types` — **minor**: entry-reachable growth. Two new exported interfaces + plus a widened method return on `DataSource`, all reachable from the package entry. +- `@object-ui/data-objectstack` — **minor**, measured rather than assumed. Its emitted + `dist/index.d.ts` is **not** byte-identical after the swap: the two `interface` blocks + leave the file and are replaced by a re-export from `@object-ui/types` (121.61 KB to + 120.25 KB). Both names remain in the public export list, so no importer breaks, but the + declaration genuinely moved and the emitted types now depend on `@object-ui/types` for + it — that is a minor, not a patch. diff --git a/packages/data-objectstack/src/deleteViewContract.types.test.ts b/packages/data-objectstack/src/deleteViewContract.types.test.ts new file mode 100644 index 0000000000..fcec22bc7f --- /dev/null +++ b/packages/data-objectstack/src/deleteViewContract.types.test.ts @@ -0,0 +1,246 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import { describe, it, expect, vi } from 'vitest'; +import type { DataSource, DeleteViewResult, ViewHomeDeleteOutcome } from '@object-ui/types'; +import { + ObjectStackAdapter, + type DeleteViewResult as AdapterDeleteViewResult, + type ViewHomeDeleteOutcome as AdapterViewHomeDeleteOutcome, +} from './index'; + +/** + * The shared `DataSource` contract carries `deleteView`'s per-home outcomes + * (#4564). + * + * #4479 / PR #4562 widened THIS adapter's `deleteView` to return + * `DeleteViewResult { deleted, draft?, published? }`, but the shared interface + * in `@object-ui/types` still declared the narrow `{ deleted: boolean }`. The + * adapter's wider return is assignable to the narrower declaration, so nothing + * failed to compile — which is exactly what made the gap invisible: a consumer + * reaching the adapter THROUGH `DataSource` was handed a type that had already + * thrown the per-home outcomes away. The one real call site today (app-shell's + * `ObjectView` delete handler) reads nothing off the receipt, so the loss was + * latent rather than broken. + * + * #4564's ruling moved the canonical shapes DOWN to `@object-ui/types` beside + * the interface (the dependency runs data-objectstack -> types, so the shapes + * could not be imported upward), widened `DataSource.deleteView?`'s declared + * return to `DeleteViewResult`, and left this package re-exporting both names + * so every existing importer keeps compiling. + * + * ## Where these pins get their colour + * + * Most of this file is COMPILE-time. That is not a weaker pin, it is the only + * pin that can observe this defect: the bug was never a wrong value at runtime, + * it was a declaration that could not describe the value. `vitest` transpiles + * with esbuild and erases types, so the runtime case below passed just as + * happily against the narrow declaration — its `receipt.draft` read was a + * `tsc` error and a green test at the same time. The colour therefore comes + * from `pnpm --filter @object-ui/data-objectstack type-check`, and this + * package's `tsconfig.json` includes its whole `src/**` (tests included), so + * these assertions are checked there and by CI's Type Check job with no + * separate `tsconfig.typetests.json` — the same property `queryDataset.test.ts` + * and `spec-symbol-batch6.test.ts` document and rely on. + * + * ## The discrimination control + * + * `NarrowLegacyResult` below is `deleteView`'s declared return BEFORE #4564, + * kept as a live control rather than described in prose. Every "the contract + * carries the per-home outcomes" assertion is paired with the control failing + * to carry them. Undo the move and the paired assertions go red together, + * which is what stops this file from degrading into a restatement of whatever + * the interface happens to say. + */ + +const VIEW = 'account.my_pipeline'; + +/** Framework receipt: a draft row existed and was discarded. */ +const DRAFT_DISCARDED = { + success: true, + reset: true, + seq: 41, + message: `Draft discarded — view/${VIEW}. [seq=41]`, +}; + +/** Framework receipt: no published row. A 200 carrying `reset:false`, not a 404. */ +const NO_PUBLISHED = { + success: true, + reset: false, + message: `No view '${VIEW}' found — nothing to delete.`, +}; + +/** + * The smallest adapter that can answer both view-delete addresses. + * + * Deliberately NOT a second copy of `deleteView.homes.test.ts`'s harness: that + * file owns the behaviour (15 pins over both homes, the ordering, the failure + * paths) and stays untouched. This one exists only to drive one call through a + * `DataSource`-typed handle, so the per-home read below is served by the shared + * declaration and by nothing else. + */ +function makeAdapter(): ObjectStackAdapter { + const baseUrl = 'http://test.local'; + const json = (body: unknown) => + new Response(JSON.stringify(body), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); + + const fetchImpl = vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + const url = String(input); + if (url.includes('/meta/view/') && (init?.method ?? 'GET') === 'DELETE') { + return json(url.includes('state=draft') ? DRAFT_DISCARDED : NO_PUBLISHED); + } + return json({ success: true, data: { capabilities: {}, routes: {} } }); + }); + + const adapter = new ObjectStackAdapter({ baseUrl, fetch: fetchImpl }); + const poke = adapter as any; + poke.connected = true; + poke.connectionState = 'connected'; + poke.metadataCache = { + get: async (_key: string, loader: () => Promise) => loader(), + invalidate: () => undefined, + getCachedSync: () => undefined, + getStats: () => ({}), + }; + return adapter; +} + +/* -------------------------------------------------------------------------- */ +/* Compile-time vocabulary — the repo's idiom (queryDataset.test.ts). */ +/* -------------------------------------------------------------------------- */ + +type Assert = T; +type Equal = (() => T extends A ? 1 : 2) extends () => T extends B ? 1 : 2 + ? true + : false; +type HasKey = K extends keyof T ? true : false; + +/** The shared contract's declared return, read off the interface itself. */ +type ContractDeleteView = NonNullable; +type ContractReturn = Awaited>; + +/** + * `DataSource.deleteView`'s declared return BEFORE #4564 — the control. + * Kept as a type, not a comment, so "the contract used to lose the per-home + * outcomes" is an assertion the compiler re-checks on every run. + */ +interface NarrowLegacyResult { + deleted: boolean; +} + +describe('DataSource.deleteView declares the per-home outcomes (#4564)', () => { + it('the shared contract returns the canonical DeleteViewResult, not a restatement', () => { + // `Equal` rather than a two-way `extends`: a structurally overlapping + // re-declaration in `types` would satisfy mutual assignability while + // drifting, and a hand copy is precisely what the one-resolver rule and + // this card's ruling rejected in favour of moving the declaration. + type _IsTheCanonicalShape = Assert>; + + expect(true).toBe(true); + }); + + it('carries `draft` and `published`, where the pre-#4564 declaration carried neither', () => { + type _CarriesDraft = Assert, true>>; + type _CarriesPublished = Assert, true>>; + + // The control, and the whole reason this file is a pin rather than an echo: + // the shape the interface used to declare cannot answer either read. If the + // move is reverted, the two assertions above go red and these two stay + // green — the difference between them IS #4564's delta. + type _ControlHasNoDraft = Assert, false>>; + type _ControlHasNoPublished = Assert, false>>; + type _ContractIsNotTheNarrowShape = Assert< + Equal, false> + >; + + expect(true).toBe(true); + }); + + it('types the per-home outcomes as ViewHomeDeleteOutcome, optional on both homes', () => { + type _DraftIsTheOutcome = Assert>; + type _PublishedIsTheOutcome = Assert< + Equal + >; + // `deleted` is unchanged — the growth is additive, so a consumer reading + // only `deleted` is untouched by the widening. + type _DeletedUnchanged = Assert>; + + expect(true).toBe(true); + }); + + it('MUST NOT CHANGE: deleteView stays OPTIONAL, with the same two parameters', () => { + // The widening is to the RETURN only. An adapter that never implemented + // `deleteView` must stay a legal `DataSource`. + type _StillOptional = Assert; + type _SameParameters = Assert, [string, string]>>; + + expect(true).toBe(true); + }); +}); + +describe('the data-objectstack spellings survive the move (#4564)', () => { + it('re-exports both names as the SAME declaration, not a structural twin', () => { + // The re-export contract: every importer that PR #4562 left pointing at + // `@object-ui/data-objectstack` keeps compiling, and keeps getting the type + // the shared interface now speaks. `Equal` is the assertion that matters — + // two identical-looking declarations are mutually assignable, so only + // identity can tell a re-export from a copy that will drift. + type _ResultIsTheTypesSpelling = Assert>; + type _OutcomeIsTheTypesSpelling = Assert< + Equal + >; + + expect(true).toBe(true); + }); + + it('the adapter method still satisfies the widened contract', () => { + const adapter = makeAdapter(); + + // Explicit, because `class ObjectStackAdapter implements DataSource` would + // have gone on compiling with the NARROW declaration too — a wider return + // is assignable to a narrower one, which is how the gap survived #4562's + // review in the first place. This states the direction the card cares + // about instead of inheriting it from the class heritage clause. + const deleteView = adapter.deleteView.bind(adapter) satisfies ContractDeleteView; + expect(typeof deleteView).toBe('function'); + + type _AdapterAssignableToContract = Assert< + ObjectStackAdapter['deleteView'] extends ContractDeleteView ? true : false + >; + // ...and no longer merely assignable: the two returns are now one type, so + // the adapter cannot widen further without the contract following it. + type _ReturnsAreOneType = Assert< + Equal>, ContractReturn> + >; + + expect(true).toBe(true); + }); +}); + +describe('a consumer holding only the shared DataSource (#4564)', () => { + it('reads the per-home outcomes the adapter actually returns', async () => { + const adapter = makeAdapter(); + + // The narrowing that makes this a #4564 pin rather than a duplicate of + // `deleteView.homes.test.ts`: the receipt below is typed by `DataSource`, + // not by `ObjectStackAdapter`. Before the move this assignment compiled + // just as well and threw the per-home outcomes away on the way through. + const ds: DataSource = adapter; + + const receipt = await ds.deleteView!('account', VIEW); + + // Draft-only view: the draft home held the row, the published one did not. + expect(receipt.deleted).toBe(true); + expect(receipt.draft?.removed).toBe(true); + expect(receipt.published?.removed).toBe(false); + expect(receipt.draft?.message).toContain('Draft discarded'); + }); +}); diff --git a/packages/data-objectstack/src/index.ts b/packages/data-objectstack/src/index.ts index 0681d3b554..d154c65c2a 100644 --- a/packages/data-objectstack/src/index.ts +++ b/packages/data-objectstack/src/index.ts @@ -19,6 +19,12 @@ import { import type { AnalyticsResult, DatasetSelection } from '@objectstack/spec/contracts'; import type { DataSource, + // #4564 — the canonical `deleteView` receipt shapes, declared beside the + // `DataSource` interface they belong to. Imported for the local uses below + // and re-exported under the same names further down, so this package's + // existing importers are untouched by the move. + DeleteViewResult, + ViewHomeDeleteOutcome, BatchTransactionOperation, DataSourceMutationEvent, QueryParams, @@ -1054,41 +1060,26 @@ function unwrapViewDraft(resp: unknown): Record | null { } /** - * Outcome of {@link ObjectStackAdapter.deleteView}'s delete against ONE of a - * view's two homes — the pending draft, or the published overlay (#4479). - */ -export interface ViewHomeDeleteOutcome { - /** - * True when this home held a row and the server removed it. False is the - * "there was nothing here" answer, which is a success, not a failure: the - * framework reports a missing home as a 200 carrying `reset:false`. - */ - removed: boolean; - /** The server receipt's `reset` flag, when it sent one. */ - reset?: boolean; - /** The server receipt's human-readable message, when it sent one. */ - message?: string; -} - -/** - * Receipt for {@link ObjectStackAdapter.deleteView} (#4479). + * `deleteView`'s receipt shapes, RE-EXPORTED from `@object-ui/types` (#4564). + * + * #4479 first declared both here, because this adapter is where the widened + * receipt is produced. That left the shared `DataSource.deleteView?` unable to + * describe it: the dependency runs this package -> `@object-ui/types` and never + * the other way, so the interface kept the narrow `{ deleted: boolean }` and a + * consumer reaching this adapter THROUGH `DataSource` was handed a type with + * the per-home outcomes already discarded. Nothing failed to compile — a wider + * return is assignable to a narrower declaration — which is precisely why the + * gap was silent until #4564 measured it. * - * The per-home fields are ADDITIVE over the original `{ deleted: boolean }`: - * a caller that only reads `deleted` is unaffected, and one that needs to tell - * "draft gone, overlay left" from "both gone" now can. + * So the canonical declarations now live beside the interface they belong to, + * in `packages/types/src/data.ts`, and this package re-exports them under the + * SAME names: every importer PR #4562 left pointing at + * `@object-ui/data-objectstack` keeps compiling, and now gets the very type the + * shared contract speaks rather than a structural twin of it. Pinned by + * `deleteViewContract.types.test.ts`, which asserts type IDENTITY — mutual + * assignability would have been satisfied by a copy that was already drifting. */ -export interface DeleteViewResult { - /** - * True only when no home is left serving the view AND at least one home - * actually held a row. A view that existed in neither home answers `false` - * — the same answer that shape has always given. - */ - deleted: boolean; - /** Outcome against the pending draft (`?state=draft`). */ - draft?: ViewHomeDeleteOutcome; - /** Outcome against the published overlay. */ - published?: ViewHomeDeleteOutcome; -} +export type { DeleteViewResult, ViewHomeDeleteOutcome } from '@object-ui/types'; /** * Read one delete receipt into a {@link ViewHomeDeleteOutcome}. diff --git a/packages/types/src/data.ts b/packages/types/src/data.ts index 9c6648db43..51dc22b56b 100644 --- a/packages/types/src/data.ts +++ b/packages/types/src/data.ts @@ -251,6 +251,52 @@ export interface GlobalSearchResult { hits: GlobalSearchHit[]; } +/** + * Outcome of a {@link DataSource.deleteView} delete against ONE of a view's two + * homes — the pending draft, or the published overlay (objectui#4479). + */ +export interface ViewHomeDeleteOutcome { + /** + * True when this home held a row and the server removed it. False is the + * "there was nothing here" answer, which is a success, not a failure: the + * framework reports a missing home as a 200 carrying `reset:false`. + */ + removed: boolean; + /** The server receipt's `reset` flag, when it sent one. */ + reset?: boolean; + /** The server receipt's human-readable message, when it sent one. */ + message?: string; +} + +/** + * Receipt for {@link DataSource.deleteView} (objectui#4479, moved here by + * objectui#4564). + * + * The per-home fields are ADDITIVE over the original `{ deleted: boolean }`: + * a caller that only reads `deleted` is unaffected, and one that needs to tell + * "draft gone, overlay left" from "both gone" now can. + * + * Declared HERE rather than in the adapter that first returned it, because the + * dependency runs `@object-ui/data-objectstack` -> `@object-ui/types` and never + * the other way. While the shape lived downstream, the interface could only + * declare the narrow `{ deleted: boolean }`, and a consumer reaching an adapter + * THROUGH `DataSource` was handed a type that had already discarded the + * per-home outcomes — the adapter's wider return being assignable to the + * narrower declaration is what kept that silent (objectui#4564). + */ +export interface DeleteViewResult { + /** + * True only when no home is left serving the view AND at least one home + * actually held a row. A view that existed in neither home answers `false` + * — the same answer that shape has always given. + */ + deleted: boolean; + /** Outcome against the pending draft (`?state=draft`). */ + draft?: ViewHomeDeleteOutcome; + /** Outcome against the published overlay. */ + published?: ViewHomeDeleteOutcome; +} + export interface DataSource { /** * Fetch multiple records. @@ -534,8 +580,16 @@ export interface DataSource { * Delete an overlay view. Routes to `DELETE /api/v1/meta/view/:name`, * which resets to the artifact default if one exists or removes the * overlay entirely if it was a user-created view. - */ - deleteView?(objectName: string, viewName: string): Promise<{ deleted: boolean }>; + * + * A view has TWO homes — the pending per-item draft (`?state=draft`) and the + * published overlay — and "remove this view" is satisfied only when neither + * is left serving it (objectui#4479). The receipt is + * {@link DeleteViewResult}: `deleted` is the unchanged single-bit answer, and + * the optional per-home outcomes let a caller tell a partial result ("draft + * gone, overlay left") from a complete one instead of having it rounded up. + * Implementations that address only one home report the other absent. + */ + deleteView?(objectName: string, viewName: string): Promise; /** diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index c1cb5cb685..5f4c544c4e 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -281,6 +281,8 @@ export type { QueryParams, QueryResult, DataSource, + DeleteViewResult, + ViewHomeDeleteOutcome, GlobalSearchHit, GlobalSearchResult, BatchRef,