diff --git a/.changeset/7323-adapter-factory-return.md b/.changeset/7323-adapter-factory-return.md new file mode 100644 index 000000000..2f4925d08 --- /dev/null +++ b/.changeset/7323-adapter-factory-return.md @@ -0,0 +1,79 @@ +--- +'@object-ui/data-objectstack': minor +--- + +`createObjectStackAdapter` declares the adapter it returns, not the shared `DataSource` +interface (objectui#7323). + +The factory returned `new ObjectStackAdapter(config)` while declaring `DataSource`. +A wider value is assignable to a narrower annotation, so nothing ever failed to compile +— the loss was entirely on the reading side. Measured against the shipped +`dist/index.d.ts` with the doc-snippet gate's own compiler options, nine reads through +`ReturnType` failed with TS2339: `getClient`, +`getCacheStats`, `invalidateCache`, `clearCache`, `getConnectionState`, `isConnected`, +`onConnectionStateChange`, `onBatchProgress` and `setSystemCapabilities`. Eight of those +nine reads are on this package's README API Reference list, and four whole README +sections are built on them; the ninth measured read is the one the factory's own JSDoc +links to (`[ADR-0066] See {@link ObjectStackAdapter.setSystemCapabilities}`). The README +list is itself **nine** adapter-only members, not eight — `connect()` is adapter-only +too and was documented all along; it simply was not one of the reads the card's +reproduction measured. So the file's own doc comment pointed the reader at a method its +declared return hid, and the two documented ways to obtain the same object — the factory +and `new ObjectStackAdapter(…)` — handed back different type surfaces. + +**What the declared return now is: the whole class, not those nine reads.** The nine +above are what the reproduction measured, not the size of this change. The factory's +declared return is now `ObjectStackAdapter` itself, so **every public member of the +class** is part of what the factory promises. Against `DataSource` that is **20** +members, not nine — `tsc`-computed as +`Exclude, keyof DataSource>`: `clearCache`, +`connect`, `getCacheStats`, `getCached`, `getClient`, `getConnectionState`, +`getDiscovery`, `getItems`, `invalidateCache`, `invalidateViewKeys`, `isConnected`, +`listImportMappings`, `onBatchProgress`, `onConnectionStateChange`, `onSaveAdvisory`, +`onWriteWarning`, `probeAppAccess`, `queryDataset`, `setSystemCapabilities`, +`updateDashboard`. The eleven past the documented nine were already in the shipped class +type — none is `@internal` or `@deprecated`, `stripInternal` is not set, and all were +already reachable through `new ObjectStackAdapter(…)` and through every +`ObjectStackAdapter`-typed seam in `@object-ui/react` and `app-shell` — so what widens +here is what the **factory declares**, not what the package ships. Two are escape-hatch +shaped and worth knowing before building on them: `getCached(key)` is a raw cache read, +and `getDiscovery()` reaches an internal property of the underlying `ObjectStackClient`. + +**Branch taken: A (widen the factory's declared return), and why.** The card offered +three. B — moving caching, connection state and batch progress onto `DataSource` — was +rejected because those are this adapter's concerns, not every data source's; every other +`DataSource` implementation would then declare members it does not have. C — documenting +a cast — teaches a cast around a declaration that is merely narrower than the value, +which is the opposite of `declared = enforced`. A is one line and makes declared match +shipped for every documented member at once. + +Two questions decided the shape and both were answered from the code before the diff. +`ObjectStackAdapter` was **already** exported from the package's only entry +(`src/index.ts`, tsup's single entry; the class is in the shipped `dist/index.d.ts` +export list, two pin tests assert the exported spelling, and `apps/console` re-exports it +by name) — so widening the return exports nothing by implication. And the narrow return +was **not** a deliberate swappability guarantee: no comment, ADR or test pinned it, and +the commit that added `autoReconnect` / `maxReconnectAttempts` / `reconnectDelay` to the +factory's own config bag left the members that observe those features off the factory's +declared return in the same change. + +**One caller shape breaks: a structural stand-in for the factory's return.** A +hand-written object literal annotated `ReturnType` no +longer satisfies that type, because it is now a class with private members (TS2740) — +annotate such a fake as `DataSource` instead, which is what it was standing in for. +Nothing else moves: a wider return is assignable to the narrower annotation, so +`const ds: DataSource = createObjectStackAdapter(…)` keeps compiling and keeps giving +the narrow surface to anyone who wants it. + +The README's note saying the page could not yet teach the factory's shape is removed, and +the four sections built on the adapter-only members (Metadata Caching, Connection State +Monitoring, Batch Operation Progress, Troubleshooting → Cache Issues) now continue from +Basic Setup's `createObjectStackAdapter(…)` call instead of declaring the class by hand. +The docs-site page `content/docs/utilities/data-objectstack.mdx` is corrected the same +way: its prose, its factory signature fragment and its "hold the class type to reach +these" section described the old narrow return, and its Mutations and Troubleshooting +examples told the reader to construct the class by hand to reach members the factory now +declares. `src/adapterFactoryReturn.types.test.ts` pins the card's TS2339 reproduction +inverted, +with two controls: the adapter-only members stay absent from `DataSource` (fires on +option B), and the widened return stays assignable to `DataSource` (swappability kept). diff --git a/content/docs/utilities/data-objectstack.mdx b/content/docs/utilities/data-objectstack.mdx index e84bb3d0c..c0df1edf6 100644 --- a/content/docs/utilities/data-objectstack.mdx +++ b/content/docs/utilities/data-objectstack.mdx @@ -50,9 +50,11 @@ const dataSource = createObjectStackAdapter({ }); ``` -`createObjectStackAdapter` returns a `DataSource` — the same universal interface -every ObjectUI renderer consumes. `new ObjectStackAdapter(config)` is the class -form of the same thing. +`createObjectStackAdapter` returns an `ObjectStackAdapter` — the concrete adapter +class, which implements `DataSource`, the universal interface every ObjectUI +renderer consumes. `new ObjectStackAdapter(config)` is the class form of the same +thing and has the same type. Annotate the value as `DataSource` wherever you want +only the universal surface. ### 2. Inject it at the renderer boundary @@ -106,11 +108,12 @@ full table of which blocks honour which keys. ### `createObjectStackAdapter` -Factory returning a `DataSource` backed by an ObjectStack backend. +Factory returning an `ObjectStackAdapter` — the concrete adapter class, which +implements `DataSource` — backed by an ObjectStack backend. **Config:** -{/* doc-snippet: fragment — a SIGNATURE excerpt of the factory, quoted from its declaration so the config members can be annotated one by one: a `function` declaration with no body and a `DataSource` return type this reference block does not import (measured: TS2391 x1, TS2304 x1). Checked against the shipped `packages/data-objectstack/dist/index.d.ts`: every member listed here is declared there with the same type */} +{/* doc-snippet: fragment — a SIGNATURE excerpt of the factory, quoted from its declaration so the config members can be annotated one by one: a `function` declaration with no body and an `ObjectStackAdapter` return type this reference block does not import (measured: TS2391 x1, TS2304 x1). Because the block is DECLARED, this gate never compiles it, so the agreement with the shipped `packages/data-objectstack/dist/index.d.ts` — every config member declared there with the same type, and the return type — is hand-checked at each edit, not gate-enforced; that gap is why the return type here outlived the change that widened it (objectui#7323) */} ```typescript function createObjectStackAdapter(config: { /** ObjectStack server base URL */ @@ -132,7 +135,7 @@ function createObjectStackAdapter(config: { autoReconnect?: boolean; // default true maxReconnectAttempts?: number; // default 3 reconnectDelay?: number; // default 1000 ms -}): DataSource; +}): ObjectStackAdapter; ``` **Example:** @@ -161,20 +164,43 @@ const dataSource = createObjectStackAdapter({ baseUrl: 'https://api.exampl ### `ObjectStackAdapter` -The class behind the factory. `new ObjectStackAdapter(config)` takes the same -config, but its declared type is the **concrete adapter** rather than the -`DataSource` interface the factory returns — which matters, because part of the -adapter's surface is not on that interface: +The class behind the factory — and the type the factory declares. `new +ObjectStackAdapter(config)` and `createObjectStackAdapter(config)` take the same +config and produce the same type, so the two forms are interchangeable: ```typescript -import { ObjectStackAdapter } from '@object-ui/data-objectstack'; +import { createObjectStackAdapter, ObjectStackAdapter } from '@object-ui/data-objectstack'; type User = { id: string; name: string; email: string }; -const adapter = new ObjectStackAdapter({ baseUrl: 'https://api.example.com' }); +// Same declared type, either way. +const fromFactory = createObjectStackAdapter({ baseUrl: 'https://api.example.com' }); +const fromClass = new ObjectStackAdapter({ baseUrl: 'https://api.example.com' }); ``` -**On the `DataSource` interface** (available from either form): +**What changed.** This section used to tell you to hold the class type to +reach the members listed under *Beyond `DataSource`* below, because the factory +declared `DataSource` while returning `new ObjectStackAdapter(config)`: the +value always carried those members, only the declared type hid them +(objectui#7323). That distinction is gone. If you switched to +`new ObjectStackAdapter(...)` solely to reach a cache, connection-state or batch +method, you can switch back to the factory — nothing about the value changes, and +neither does its type. The class name is still worth importing when you need +something to annotate with. + +Narrowing still works, and is still the right shape for a prop, a field or a test +double that must accept **any** adapter: + +```typescript +import type { DataSource } from '@object-ui/types'; +import { createObjectStackAdapter } from '@object-ui/data-objectstack'; + +// The universal surface only, by annotation. +const dataSource: DataSource = createObjectStackAdapter({ baseUrl: 'https://api.example.com' }); +``` + +**On the `DataSource` interface** (available from either form, and from any other +adapter): - `find(resource, params?)` - Query multiple records - `findOne(resource, id, params?)` - Get a single record by ID @@ -184,14 +210,18 @@ const adapter = new ObjectStackAdapter({ baseUrl: 'https://api.example.com - `getObjectSchema(objectName)` - Fetch schema metadata (cached) - `bulk?(resource, operation, data)` - Batch create/update/delete on one object - `batchTransaction?(operations)` - Cross-object atomic batch (master-detail) +- `onMutation?(listener)` - Subscribe to create/update/delete events -`bulk` and `batchTransaction` are **optional** members of `DataSource`: not every -adapter implements them, so through a `DataSource`-typed value they must be -feature-detected (`typeof dataSource.bulk === 'function'`). `ObjectStackAdapter` -implements both unconditionally, so a value held at the class type calls them -directly. +`bulk`, `batchTransaction` and `onMutation` are **optional** members of +`DataSource`: not every adapter implements them, so through a `DataSource`-typed +value they must be feature-detected (`typeof dataSource.bulk === 'function'`). +`ObjectStackAdapter` implements all three unconditionally, and the factory +declares the class, so a value from either form calls them directly. (`onMutation` +was listed under *Adapter-only* here until objectui#7323; it is optional on +`DataSource`, not absent from it.) -**Adapter-only** (hold the class type to reach these): +**Beyond `DataSource`** (declared on `ObjectStackAdapter`, so reachable from +either form): - `connect()` - Establish the connection (called lazily by every operation) - `getCacheStats()` / `invalidateCache(key?)` / `clearCache()` - Cache control @@ -199,7 +229,14 @@ directly. - `getConnectionState()` / `isConnected()` - Connection introspection - `onConnectionStateChange(listener)` - Subscribe to state changes (returns unsubscribe) - `onBatchProgress(listener)` - Subscribe to bulk progress (returns unsubscribe) -- `onMutation(listener)` - Subscribe to create/update/delete events + +Those six bullets cover nine members, and they are the documented subset. +`ObjectStackAdapter` declares **20** members beyond `DataSource` in all +(`Exclude, keyof DataSource>`); the +other eleven are lower-level seams — client discovery, cache-key invalidation, +dataset and dashboard access, advisory subscriptions and capability configuration +among them — which this page does not document and which carry no compatibility +promise here. ### Per-element data binding @@ -369,15 +406,16 @@ await dataSource.delete('user', user.id); Batch writes on one object go through `bulk`, and cross-object writes that must commit or roll back together go through `batchTransaction`. Both are optional on -the `DataSource` interface, so hold the adapter at its class type (or -feature-detect) before calling them: +the `DataSource` interface, so a value you annotated as `DataSource` still has to +feature-detect them — but the adapter implements both unconditionally and the +factory declares the adapter, so the value from Quick Start calls them directly: ```typescript -import { ObjectStackAdapter } from '@object-ui/data-objectstack'; +import { createObjectStackAdapter } from '@object-ui/data-objectstack'; type User = { id: string; name: string; email: string }; -const adapter = new ObjectStackAdapter({ baseUrl: 'https://api.example.com' }); +const adapter = createObjectStackAdapter({ baseUrl: 'https://api.example.com' }); await adapter.bulk('user', 'create', [ { name: 'Alice', email: 'alice@example.com' }, @@ -463,17 +501,18 @@ An `AuthenticationError` (code `AUTHENTICATION_ERROR`, status 401) means the `token` passed to `createObjectStackAdapter` was missing, expired or rejected. Check the connection state and the values you passed in: -Connection introspection lives on the adapter class, not on the `DataSource` -interface, so hold it at the class type: +Connection introspection lives on the adapter, not on the `DataSource` interface +— and the factory declares the adapter, so the value from Quick Start reaches it +without a second construction: ```typescript -import { ObjectStackAdapter } from '@object-ui/data-objectstack'; +import { createObjectStackAdapter } from '@object-ui/data-objectstack'; -const adapter = new ObjectStackAdapter({ baseUrl: 'https://api.example.com' }); +const dataSource = createObjectStackAdapter({ baseUrl: 'https://api.example.com' }); -console.log(adapter.getConnectionState()); // 'connected' | 'error' | ... +console.log(dataSource.getConnectionState()); // 'connected' | 'error' | ... -adapter.onConnectionStateChange((event) => { +dataSource.onConnectionStateChange((event) => { if (event.error) console.error('Connection error:', event.error); }); ``` diff --git a/packages/data-objectstack/README.md b/packages/data-objectstack/README.md index dff473e0a..d517ea857 100644 --- a/packages/data-objectstack/README.md +++ b/packages/data-objectstack/README.md @@ -44,16 +44,6 @@ function App() { } ``` -> **Reaching the adapter-only API from TypeScript.** `createObjectStackAdapter` -> declares `DataSource` as its return type, so the members below that belong to the -> adapter rather than to every data source — `getClient`, the cache methods, the -> connection-state and batch-progress subscriptions — are not on the type the factory -> hands back, even though they are on the object it hands back. Until -> [#7323](https://github.com/objectstack-ai/objectui/issues/7323) is settled, hold the -> adapter as `ObjectStackAdapter` (the exported class, whose constructor is documented -> under **API Reference** below) wherever you use those members; the examples in this -> README do exactly that. - ### Advanced Configuration ```typescript @@ -258,9 +248,9 @@ await dataSource.find('users', { The adapter includes built-in metadata caching to improve performance when fetching schemas: ```typescript -import type { ObjectStackAdapter } from '@object-ui/data-objectstack'; +import { createObjectStackAdapter } from '@object-ui/data-objectstack'; -declare const dataSource: ObjectStackAdapter; +const dataSource = createObjectStackAdapter({ baseUrl: 'https://api.example.com' }); // Get cache statistics const stats = dataSource.getCacheStats(); @@ -288,9 +278,9 @@ dataSource.clearCache(); The adapter provides real-time connection state monitoring with automatic reconnection: ```typescript -import type { ObjectStackAdapter } from '@object-ui/data-objectstack'; +import { createObjectStackAdapter } from '@object-ui/data-objectstack'; -declare const dataSource: ObjectStackAdapter; +const dataSource = createObjectStackAdapter({ baseUrl: 'https://api.example.com' }); // Monitor connection state changes const unsubscribe = dataSource.onConnectionStateChange((event) => { @@ -336,9 +326,9 @@ The adapter automatically attempts to reconnect on connection failures: Track progress of bulk operations in real-time: ```typescript -import type { ObjectStackAdapter } from '@object-ui/data-objectstack'; +import { createObjectStackAdapter } from '@object-ui/data-objectstack'; -declare const dataSource: ObjectStackAdapter; +const dataSource = createObjectStackAdapter({ baseUrl: 'https://api.example.com' }); declare const largeDataset: Array>; @@ -712,9 +702,9 @@ const dataSource = createObjectStackAdapter({ #### Cache Issues ```typescript -import type { ObjectStackAdapter } from '@object-ui/data-objectstack'; +import { createObjectStackAdapter } from '@object-ui/data-objectstack'; -declare const dataSource: ObjectStackAdapter; +const dataSource = createObjectStackAdapter({ baseUrl: 'https://api.example.com' }); // Clear cache if stale data is being returned dataSource.clearCache(); diff --git a/packages/data-objectstack/src/adapterFactoryReturn.types.test.ts b/packages/data-objectstack/src/adapterFactoryReturn.types.test.ts new file mode 100644 index 000000000..f64b242a8 --- /dev/null +++ b/packages/data-objectstack/src/adapterFactoryReturn.types.test.ts @@ -0,0 +1,162 @@ +/** + * 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 } from 'vitest'; +import type { DataSource } from '@object-ui/types'; +import { createObjectStackAdapter, ObjectStackAdapter } from './index'; + +/** + * `createObjectStackAdapter`'s declared return is the adapter, not the shared + * `DataSource` interface (#7323). + * + * The factory declared `DataSource` while returning `new ObjectStackAdapter`. + * A wider value is assignable to a narrower annotation, so nothing failed to + * compile — the loss was entirely on the reading side: every adapter-only + * member was erased from the type the factory handed back, while staying on the + * object it handed back. Measured on the shipped `dist/index.d.ts` before the + * fix, nine reads through `ReturnType` failed + * with TS2339: `getClient`, `getCacheStats`, `invalidateCache`, `clearCache`, + * `getConnectionState`, `isConnected`, `onConnectionStateChange`, + * `onBatchProgress`, `setSystemCapabilities`. Eight of those are exactly the + * members the package README's API Reference documents, and the ninth is the + * one the factory's own JSDoc links to. + * + * ## Where these pins get their colour + * + * Most of this file is COMPILE-time, which is the only place this defect is + * observable: the values were always there, so every runtime test passed + * against the narrow declaration too. `vitest` transpiles with esbuild and + * erases types, so the colour comes from + * `pnpm --filter @object-ui/data-objectstack type-check` — this package's + * `tsconfig.json` includes its whole `src/**` (tests included), the same + * property `deleteViewContract.types.test.ts` documents and relies on. + * + * ## The controls + * + * Two, and they answer different questions. + * + * 1. `_NotOnDataSource` — the adapter-only members are ABSENT from the shared + * `DataSource` interface. This is what makes the reads below a statement + * about the FACTORY's return rather than a statement about every data + * source. It also fires on option B of the card (moving caching, + * connection state and batch progress onto `DataSource` so every + * implementation has to declare them) — the shape #7323 argues against. + * 2. `_StillADataSource` — the widened return is still assignable to + * `DataSource`. The triage's open question was whether the narrow return + * was a deliberate swappability guarantee; this states that widening did + * not cost it, so a caller who wants the narrow surface still just writes + * `const ds: DataSource = createObjectStackAdapter(…)`. + * + * Both controls are independent of the return annotation, so a revert of the + * source change turns the reads below red and leaves these two green. + */ + +type Assert = T; +type Equal = (() => T extends A ? 1 : 2) extends () => T extends B ? 1 : 2 + ? true + : false; + +type FactoryReturn = ReturnType; + +describe('createObjectStackAdapter declares the adapter it returns (#7323)', () => { + it('exposes the adapter-only members the README documents', () => { + const dataSource = createObjectStackAdapter({ baseUrl: 'http://test.local' }); + + // The card's own TS2339 reproduction, inverted into a pin. Each read is a + // compile error the moment the declared return narrows back. They are real + // calls rather than a type-only region because every one of them is inert + // on an adapter that has never connected, so the same nine lines carry the + // runtime half too. + expect(dataSource.getClient()).toBeDefined(); + expect(dataSource.getCacheStats()).toBeDefined(); + dataSource.invalidateCache('users'); + dataSource.invalidateCache(); + dataSource.clearCache(); + expect(dataSource.getConnectionState()).toBe('disconnected'); + expect(dataSource.isConnected()).toBe(false); + expect(typeof dataSource.onConnectionStateChange(() => {})).toBe('function'); + expect(typeof dataSource.onBatchProgress(() => {})).toBe('function'); + dataSource.setSystemCapabilities(['manage_view_config']); + + // ...and the same nine through the named type, because a consumer who + // annotates a field or a hook's return writes the type, not the call. + type _HasHiddenMembers = Assert< + 'getClient' extends keyof FactoryReturn + ? 'getCacheStats' extends keyof FactoryReturn + ? 'invalidateCache' extends keyof FactoryReturn + ? 'clearCache' extends keyof FactoryReturn + ? 'getConnectionState' extends keyof FactoryReturn + ? 'isConnected' extends keyof FactoryReturn + ? 'onConnectionStateChange' extends keyof FactoryReturn + ? 'onBatchProgress' extends keyof FactoryReturn + ? 'setSystemCapabilities' extends keyof FactoryReturn + ? true + : false + : false + : false + : false + : false + : false + : false + : false + : false + >; + + // Identity, not mere assignability: two structurally similar declarations + // are mutually assignable, so only `Equal` can tell "the factory returns + // THE adapter type" from "the factory returns something adapter-shaped". + type _IsTheAdapter = Assert>>; + + expect(true).toBe(true); + }); + + it('CONTROL — the adapter-only members are not on the shared DataSource', () => { + // Fires if anyone answers #7323 by widening `DataSource` itself (option B). + type _NotOnDataSource = Assert< + 'getCacheStats' extends keyof DataSource + ? false + : 'onConnectionStateChange' extends keyof DataSource + ? false + : 'getClient' extends keyof DataSource + ? false + : true + >; + + expect(true).toBe(true); + }); + + it('CONTROL — the widened return is still a DataSource', () => { + // Swappability, the property the narrow return was suspected of protecting. + type _StillADataSource = Assert ? true : false>; + + const dataSource: DataSource = createObjectStackAdapter({ baseUrl: 'http://test.local' }); + expect(typeof dataSource.find).toBe('function'); + }); + + it('the value really carries what the declaration now promises', () => { + // Declared = shipped. Compile-time reachability is worth nothing if the + // object does not actually have the members, so this half is runtime. + const dataSource = createObjectStackAdapter({ baseUrl: 'http://test.local' }); + + for (const member of [ + 'getClient', + 'getCacheStats', + 'invalidateCache', + 'clearCache', + 'getConnectionState', + 'isConnected', + 'onConnectionStateChange', + 'onBatchProgress', + 'setSystemCapabilities', + ] as const) { + expect(typeof dataSource[member]).toBe('function'); + } + + expect(dataSource).toBeInstanceOf(ObjectStackAdapter); + }); +}); diff --git a/packages/data-objectstack/src/index.ts b/packages/data-objectstack/src/index.ts index 737d2dc37..bad31df43 100644 --- a/packages/data-objectstack/src/index.ts +++ b/packages/data-objectstack/src/index.ts @@ -5652,6 +5652,21 @@ export class ObjectStackAdapter implements DataSource { /** * Factory function to create an ObjectStack data source. + * + * The declared return is `ObjectStackAdapter`, not the shared + * `DataSource` interface: the object this hands back is an + * `ObjectStackAdapter`, and the adapter-only members the package README + * documents — `getClient`, the metadata-cache controls, the connection-state + * and batch-progress subscriptions, and `setSystemCapabilities` — live on the + * class, not on `DataSource`. Declaring the interface here narrowed all of them + * away from the factory while leaving them on the value (objectui#7323), so + * `createObjectStackAdapter(...)` and `new ObjectStackAdapter(...)` — the two + * documented ways to obtain the same object — handed back different type + * surfaces. ⛔ Do not narrow this back to `DataSource`: a caller who wants + * the narrow surface writes `const ds: DataSource = createObjectStackAdapter(…)` + * and gets it, because a wider return is assignable to the narrower annotation; + * the reverse is not recoverable at the call site without a cast. + * `adapterFactoryReturn.types.test.ts` pins both directions. * * @example * ```typescript @@ -5677,7 +5692,7 @@ export function createObjectStackAdapter(config: { reconnectDelay?: number; /** [ADR-0066] See {@link ObjectStackAdapter.setSystemCapabilities}. */ systemCapabilities?: string[]; -}): DataSource { +}): ObjectStackAdapter { return new ObjectStackAdapter(config); }