feat(types,core,data-objectstack)!: the three surviving record ids are strings (objectui#10078)

.changeset/10078-record-id-string-survivors.md

@@ -0,0 +1,59 @@
+---
+'@object-ui/types': minor
+'@object-ui/core': minor
+'@object-ui/data-objectstack': minor
+---
+
+**BREAKING** — three more record ids that the objectui#9511 ruling's enumeration left out are
+strings now (objectui#10078). `CommentSearchResult.recordId`, `RecordSubscription.recordId`
+and `DataSourceMutationEvent.id` narrow **FROM** `string | number` **TO** `string`. ⚠️ Other
+record ids in `@object-ui/types` still admit a number and are not touched here, among them
+`DetailViewSchema.recordNavigation` (`recordIds`, `onNavigate`), `FeedItem.sourceId`, and the
+`onNavigate` slots on `ObjectGridSchema`, `ObjectViewSchema` and `ListViewRuntimeProps`.
+
+```ts
+// before — compiled
+const bell: RecordSubscription = { recordId: 42, subscribed: true };
+// after — refused by the compiler; write the id the protocol carries
+const bell: RecordSubscription = { recordId: '42', subscribed: true };
+```
+
+**Why.** objectui#9511 (director batch #195 item 1, letter A) ruled that a record id is a
+string wherever **metadata** names one, and extended that, as 「the same principle」, to
+`CommentEntry.recordId` and `MentionNotification.recordId`. It did not name these three, which
+are runtime and API shapes rather than metadata. Carrying the rule to them is the `domain:spec`
+seat's inheritance decision, recorded in the claim on objectui#10078: they hold the same id and
+were left with the old spelling. `CommentSearchResult.recordId` is copied straight from
+`CommentEntry.recordId`, `RecordSubscription.recordId` names the record a notification bell is
+for, and `DataSourceMutationEvent.id` is the id the `update` / `delete` doors already take as a
+string.
+
+**What changes at runtime.** `ValueDataSource` and `ObjectStackAdapter` now emit
+`onMutation` events whose `id` is always a string. Both methods still accept a numeric id
+through their own wider parameter, and each converts it at its own boundary before emitting —
+the one place the rule says the conversion lives. An in-memory `value` source whose items
+carry numeric keys therefore announces `'42'`, not `42`. A subscriber that compared
+`event.id` against a number must compare against the string. The two in-repo subscribers
+that read `event.id` already pass it through `String(...)`, so they see no difference.
+
+**Packages that narrow without a source change.** `@object-ui/collaboration` (`useCommentSearch`
+returns `CommentSearchResult[]`) and `@object-ui/plugin-detail` (`SubscriptionToggleProps`
+carries a `RecordSubscription`) publish the narrower type through their own `.d.ts`.
+
+**What a host has to do.** Code that builds a `RecordSubscription`, reads
+`CommentSearchResult.recordId`, or implements `DataSource.onMutation` with numeric keys
+converts once, where the number enters (its own adapter or data boundary), and passes the
+string from there on. ⛔ Nothing converts silently on the reading side.
+
+**Disposition: ADR-0087 D7 — a published runtime TypeScript interface with no metadata
+surface is carried by the compiler and needs no ledger entry; ⛔ no conversion entry, ⛔ not
+a tombstone, ⛔ not an npm `major`.** None of the three has a zod mirror, none is authored into JSON, and no schema
+references them, so there is no parse-time door and nothing for a conversion table to
+rewrite: the compiler error at the numeric site is the delivery channel. The level is `minor`
+because objectui marks its own breaking changes `minor` with this banner (AGENTS.md, version
+alignment); `major` is refused outright because every publishable package sits in one `fixed`
+group pinned to the `@objectstack` major (`scripts/check-changeset-no-major.mjs`).
+
+**Pin.** `record-id-string-survivors-10078.test.ts` in `@object-ui/types` asserts all three
+members read `string` (not `any`), refuses a numeric literal at each, and keeps a
+source-text census of the three declarations with controls that prove the census can fire.

content/docs/guide/data-source.md

@@ -15,17 +15,17 @@ import type { BatchTransactionOperation, QueryParams, QueryResult } from '@objec
 
 export interface DataSource<T = unknown> {
   find(resource: string, params?: QueryParams): Promise<QueryResult<T>>;
-  findOne(resource: string, id: string | number, params?: QueryParams): Promise<T | null>;
+  findOne(resource: string, id: string, params?: QueryParams): Promise<T | null>;
   create(resource: string, data: Partial<T>): Promise<T>;
   update(
     resource: string,
-    id: string | number,
+    id: string,
     data: Partial<T>,
     opts?: { ifMatch?: string },
   ): Promise<T>;
   delete(
     resource: string,
-    id: string | number,
+    id: string,
     opts?: { ifMatch?: string },
   ): Promise<boolean>;
 

content/docs/plugins/plugin-detail.mdx

@@ -398,7 +398,7 @@ decision as a defect is how a warning channel gets ignored:
 
 ```plaintext
 {
-  recordId: string | number,
+  recordId: string,
   subscribed: boolean,
   channels?: Array<'in_app' | 'email' | 'push'>
 }

packages/core/src/adapters/ValueDataSource.ts

@@ -1180,7 +1180,9 @@ export class ValueDataSource<T = any> implements DataSource<T> {
       throw new Error(`ValueDataSource: Record with id "${id}" not found`);
     }
     this.items[index] = { ...this.items[index], ...data };
-    this.emitMutation({ type: 'update', resource: _resource, id, record: { ...this.items[index] } });
+    // The event carries the protocol's string id (objectui#10078): an inline
+    // item may hold a numeric key, and this adapter is where it converts.
+    this.emitMutation({ type: 'update', resource: _resource, id: String(id), record: { ...this.items[index] } });
     return { ...this.items[index] };
   }
 
@@ -1190,7 +1192,7 @@ export class ValueDataSource<T = any> implements DataSource<T> {
     );
     if (index === -1) return false;
     this.items.splice(index, 1);
-    this.emitMutation({ type: 'delete', resource: _resource, id });
+    this.emitMutation({ type: 'delete', resource: _resource, id: String(id) });
     return true;
   }
 

packages/data-objectstack/src/index.ts

@@ -3908,7 +3908,9 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
         data,
         opts?.ifMatch ? { ifMatch: opts.ifMatch } : undefined,
       );
-      this.emitMutation({ type: 'update', resource, id, record: { ...result.record } });
+      // String, as sent on the wire above (objectui#10078): the event states
+      // the protocol id, whatever this method's wider parameter admitted.
+      this.emitMutation({ type: 'update', resource, id: String(id), record: { ...result.record } });
       this.notifyDroppedFields('update', resource, result, id, data as Record<string, unknown>);
       return result.record;
     } catch (err) {
@@ -3947,7 +3949,7 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
       // stale — and this method, declared `Promise<boolean>`, actually resolved
       // `undefined`. Following the rename is what restores both.
       if (result.success) {
-        this.emitMutation({ type: 'delete', resource, id });
+        this.emitMutation({ type: 'delete', resource, id: String(id) });
       }
       return result.success;
     } catch (err) {

packages/types/src/__tests__/record-id-string-survivors-10078.test.ts

@@ -0,0 +1,161 @@
+/**
+ * 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.
+ */
+
+/**
+ * objectui#10078 - three more record-id declarations that the objectui#9511
+ * ruling did not enumerate are strings too.
+ *
+ * ## What is pinned
+ *
+ * Director batch #195 item 1, letter A (objectui#9511) made a record id a
+ * `string` wherever METADATA names one, and extended it as "the same
+ * principle" to `CommentEntry.recordId` / `MentionNotification.recordId`. The
+ * three below are runtime / API shapes that ruling did not name; carrying the
+ * rule to them is the `domain:spec` seat's inheritance decision in the claim on
+ * objectui#10078. Each carries the same id with the old `string | number`
+ * spelling:
+ *
+ * - `CommentSearchResult.recordId` - the record a matching comment belongs to,
+ *   copied from `CommentEntry.recordId` (already a string since objectui#9511);
+ * - `RecordSubscription.recordId` - the record a notification bell is for;
+ * - `DataSourceMutationEvent.id` - the record a `DataSource` write touched, the
+ *   same id the `update` / `delete` doors already take as a string.
+ *
+ * All three are runtime / API shapes with NO zod mirror (nothing authors them
+ * into JSON), so the compiler is the only face that can refuse a number, and
+ * this file is where that refusal is asserted.
+ *
+ * ⚠️ This file pins these three ONLY. Other record ids in this package still
+ * admit a number and are out of its scope, so a green run here says nothing
+ * about the rest of the family.
+ *
+ * ## Where each row lives - these two halves measure different things
+ *
+ * The `type _...` rows and the `@ts-expect-error` controls are ERASED before
+ * vitest loads this file. `tsc -p packages/types/tsconfig.test.json` (chained
+ * from this package's `type-check` script) is the only thing that executes
+ * them: a green vitest run says nothing about them (objectui#3009).
+ *
+ * The `it(...)` rows are a source-text census over the declarations, which reds
+ * in a tree where `tsc` is never run and names the spelling it found. Each
+ * matcher is proven to fire on a synthetic wide spelling before an absence is
+ * believed.
+ */
+
+import { describe, it, expect } from 'vitest';
+import { readFileSync } from 'node:fs';
+import path from 'node:path';
+import { fileURLToPath } from 'node:url';
+import type { CommentSearchResult, RecordSubscription } from '../views';
+import type { DataSourceMutationEvent } from '../data';
+
+/* ------------------------------------------------------------------ *
+ * Compile-time half.
+ * ------------------------------------------------------------------ */
+
+type Equal<X, Y> =
+  (<T>() => T extends X ? 1 : 2) extends (<T>() => T extends Y ? 1 : 2) ? true : false;
+type Expect<T extends true> = T;
+
+type _CommentSearchResultRecordIdIsAString = Expect<Equal<CommentSearchResult['recordId'], string>>;
+type _RecordSubscriptionRecordIdIsAString = Expect<Equal<RecordSubscription['recordId'], string>>;
+/** Optional member: `NonNullable` strips the `undefined` that `?` adds. */
+type _MutationEventIdIsAString = Expect<Equal<NonNullable<DataSourceMutationEvent['id']>, string>>;
+
+/** Stated separately so a degradation to `any` cannot pass the rows above. */
+type _CommentSearchResultRecordIdIsNotAny = Expect<Equal<Equal<CommentSearchResult['recordId'], any>, false>>;
+type _RecordSubscriptionRecordIdIsNotAny = Expect<Equal<Equal<RecordSubscription['recordId'], any>, false>>;
+type _MutationEventIdIsNotAny = Expect<Equal<Equal<DataSourceMutationEvent['id'], any>, false>>;
+
+/**
+ * Value-site controls. Widen any member back and its directive becomes UNUSED
+ * (TS2578), so each control fires for exactly the change it exists to catch.
+ * The string spellings beside them must stay legal, or the refusals would
+ * prove nothing.
+ */
+// @ts-expect-error a numeric primary key is no longer a `CommentSearchResult.recordId`
+const _numericSearchHit: CommentSearchResult = { comment: { id: 'c1', text: '', author: '', createdAt: '' }, objectName: 'account', recordId: 42 };
+// @ts-expect-error nor a `RecordSubscription.recordId`
+const _numericSubscription: RecordSubscription = { recordId: 42, subscribed: true };
+// @ts-expect-error nor a `DataSourceMutationEvent.id`
+const _numericMutation: DataSourceMutationEvent = { type: 'delete', resource: 'account', id: 42 };
+
+const _stringSearchHit: CommentSearchResult = { comment: { id: 'c1', text: '', author: '', createdAt: '' }, objectName: 'account', recordId: '42' };
+const _stringSubscription: RecordSubscription = { recordId: '42', subscribed: true };
+const _stringMutation: DataSourceMutationEvent = { type: 'delete', resource: 'account', id: '42' };
+void [_numericSearchHit, _numericSubscription, _numericMutation, _stringSearchHit, _stringSubscription, _stringMutation];
+
+/* ------------------------------------------------------------------ *
+ * Runtime half - a census over the declaration text.
+ * ------------------------------------------------------------------ */
+
+const here = path.dirname(fileURLToPath(import.meta.url));
+// packages/types/src/__tests__  ->  packages/types/src
+const SRC = path.resolve(here, '..');
+const VIEWS_TEXT = readFileSync(path.join(SRC, 'views.ts'), 'utf8');
+const DATA_TEXT = readFileSync(path.join(SRC, 'data.ts'), 'utf8');
+
+/**
+ * The declared type of `member` inside `export interface NAME ... { ... }`.
+ * The body is cut at the first line that closes the interface, so a member of
+ * the same name in a LATER interface cannot answer for this one.
+ */
+function memberType(source: string, iface: string, member: string): string | null {
+  const start = source.indexOf(`export interface ${iface}`);
+  if (start === -1) return null;
+  const end = source.indexOf('\n}', start);
+  const body = source.slice(start, end === -1 ? undefined : end);
+  const m = new RegExp(`\\n\\s*${member}\\??\\s*:\\s*([^;\\n]+);`).exec(body);
+  return m ? m[1].trim() : null;
+}
+
+const SITES: ReadonlyArray<readonly [string, string, string]> = [
+  ['views.ts', 'CommentSearchResult', 'recordId'],
+  ['views.ts', 'RecordSubscription', 'recordId'],
+  ['data.ts', 'DataSourceMutationEvent', 'id'],
+];
+
+const TEXT: Record<string, string> = { 'views.ts': VIEWS_TEXT, 'data.ts': DATA_TEXT };
+
+describe('the census instrument can fire (controls)', () => {
+  it('reads the wide spelling back off a synthetic declaration', () => {
+    const wide = 'export interface X {\n  /** doc */\n  recordId: string | number;\n  other: boolean;\n}';
+    expect(memberType(wide, 'X', 'recordId')).toBe('string | number');
+    const wideOptional = 'export interface Y {\n  id?: string | number;\n}';
+    expect(memberType(wideOptional, 'Y', 'id')).toBe('string | number');
+  });
+
+  it('does not answer for an interface from a member of a later one', () => {
+    const two = 'export interface A {\n  x: boolean;\n}\n\nexport interface B {\n  recordId: string | number;\n}';
+    expect(memberType(two, 'A', 'recordId')).toBeNull();
+  });
+
+  it('returns null rather than a false negative when the interface is absent', () => {
+    expect(memberType(VIEWS_TEXT, 'NoSuchInterface10078', 'recordId')).toBeNull();
+  });
+
+  it('reads a member the objectui#9511 ruling already narrowed (lit control)', () => {
+    expect(memberType(VIEWS_TEXT, 'CommentEntry', 'recordId')).toBe('string');
+  });
+});
+
+describe('the three surviving record ids are strings (objectui#10078)', () => {
+  for (const [file, iface, member] of SITES) {
+    it(`\`${iface}.${member}\` declares the protocol string id`, () => {
+      expect(
+        memberType(TEXT[file], iface, member),
+        [
+          `\`${iface}.${member}\` in ${file} declares a record id wider than \`string\`.`,
+          'objectui#10078 carries the objectui#9511 record-id rule (a record id is a string',
+          'wherever metadata names one) to this runtime shape; a host whose keys are',
+          'numeric converts once, at its own adapter boundary.',
+        ].join('\n'),
+      ).toBe('string');
+    });
+  }
+});

packages/types/src/data.ts

@@ -1171,8 +1171,13 @@ export interface DataSourceMutationEvent<T = any> {
   resource: string;
   /** The affected record (present for create/update) */
   record?: T;
-  /** The ID of the affected record (present for update/delete) */
-  id?: string | number;
+  /**
+   * The ID of the affected record (present for update/delete). A string, per
+   * the one record-id rule stated on {@link DataSource} (objectui#9511,
+   * objectui#10078): an adapter whose backend keys are numeric converts at its
+   * own boundary before it emits, so no subscriber has to.
+   */
+  id?: string;
 }
 
 /**

packages/types/src/views.ts

@@ -551,8 +551,8 @@ export interface CommentSearchResult {
   comment: CommentEntry;
   /** Object name the comment belongs to */
   objectName: string;
-  /** Record ID the comment belongs to */
-  recordId: string | number;
+  /** Record ID the comment belongs to. A string, per the one record-id rule (objectui#9511, objectui#10078). */
+  recordId: string;
   /** Highlighted text snippet with search term marked */
   highlight?: string;
 }
@@ -727,8 +727,8 @@ export interface Reaction {
  * subscriptions and app billing — so none of them is a replacement.
  */
 export interface RecordSubscription {
-  /** Record ID */
-  recordId: string | number;
+  /** Record ID. A string, per the one record-id rule (objectui#9511, objectui#10078). */
+  recordId: string;
   /** Whether the current user is subscribed */
   subscribed: boolean;
   /** Notification channels */