diff --git a/.changeset/11680-placeholder-lazy-stub-guard.md b/.changeset/11680-placeholder-lazy-stub-guard.md new file mode 100644 index 0000000000..0772865655 --- /dev/null +++ b/.changeset/11680-placeholder-lazy-stub-guard.md @@ -0,0 +1,12 @@ +--- +'@object-ui/components': patch +'@object-ui/core': patch +--- + +An authored `view:calendar` or `view:timeline` node loads its plugin and renders the calendar or the timeline, and a console boot no longer logs the registry's race warning for those two keys (objectui#11680). + +The console declares both views as lazy stubs, then registers a protocol placeholder for each protocol key nothing renders yet. The placeholder registrar asked the registry about loaded components only, so it read the two stubbed keys as free and took them, and the registry cleared the stubs under them. Until some other node happened to load the calendar or the timeline chunk, an authored `view:calendar` or `view:timeline` drew the dashed "Component Placeholder" box instead of the view. `registerPlaceholders()` and the eager palette placeholders now skip a key a pending lazy stub holds (`ComponentRegistry.hasLazy`). The key stays with the plugin that declared it, and `SchemaRenderer` loads that plugin the first time the node renders. A protocol key that nothing declares still gets its placeholder. + +The registry's collision warnings now name a stub by the full type it declared. A stub found under its own namespaced key was named with its namespace written twice (`view:view:calendar`), and `register`, `registerLazy` and `unregister` compared ownership against that doubled spelling. + +**Clause-②: no.** No export is added or removed, and no accepted input changes. The placeholder registrar is not exported, and the field the registry now records on a lazy stub lives on a type the package does not export. diff --git a/apps/console/src/__tests__/full-load-placeholder-race-11680.test.ts b/apps/console/src/__tests__/full-load-placeholder-race-11680.test.ts new file mode 100644 index 0000000000..e92bdcb928 --- /dev/null +++ b/apps/console/src/__tests__/full-load-placeholder-race-11680.test.ts @@ -0,0 +1,51 @@ +/** + * 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. + */ + +/** + * A full console load logs no bare-name race warning (objectui#11680). + * + * This replays the boot `main.tsx` performs: the REAL stub list + * (`../register-plugins`), then the real `registerPlaceholders()`. The + * placeholder registrar used to read a key held by a pending stub as free, so + * it raced the console's `view:calendar` and `view:timeline` stubs on every + * boot. Reading the real list is what makes this pin catch the NEXT console + * stub on a protocol key, which a replay of two named keys cannot. + * + * The capture is installed in `vi.hoisted`, ahead of the imports, because the + * stubs register while `../register-plugins` is imported. + */ +import { describe, it, expect, vi, afterAll } from 'vitest'; + +const race = vi.hoisted(() => { + const original = console.warn; + const seen: string[] = []; + console.warn = (...args: unknown[]) => { + if (typeof args[0] === 'string' && args[0].includes('bare-name fallback is being overwritten')) seen.push(args[0]); + else original(...args); + }; + return { seen, restore: () => { console.warn = original; } }; +}); + +import { ComponentRegistry } from '@object-ui/core'; +import '../register-plugins'; +import { registerPlaceholders } from '@object-ui/components'; + +afterAll(() => race.restore()); + +describe('a full console load logs no bare-name race warning (objectui#11680)', () => { + it('declaring the stubs, then registering the placeholders, races nothing', () => { + const stubbed = ComponentRegistry.getKnownTypes().filter((key) => ComponentRegistry.hasLazy(key)); + registerPlaceholders(); + + expect(race.seen, 'a registration took a bare key another declaration holds').toEqual([]); + // Positive control: the placeholders left every stub where it was, the two + // the card measured among them, so the silence above covers those keys. + expect(stubbed).toEqual(expect.arrayContaining(['view:calendar', 'view:timeline'])); + expect(stubbed.filter((key) => !ComponentRegistry.hasLazy(key))).toEqual([]); + }); +}); diff --git a/packages/components/src/__tests__/placeholder-lazy-stub-ownership-11680.test.tsx b/packages/components/src/__tests__/placeholder-lazy-stub-ownership-11680.test.tsx new file mode 100644 index 0000000000..ea43c17fa0 --- /dev/null +++ b/packages/components/src/__tests__/placeholder-lazy-stub-ownership-11680.test.tsx @@ -0,0 +1,164 @@ +/** + * 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. + */ + +/** + * A protocol placeholder never takes a key a pending lazy stub owns + * (objectui#11680). + * + * The console declares `view:calendar` and `view:timeline` as `registerLazy` + * stubs and then calls `registerPlaceholders()`, after every real registration, + * as its `main.tsx` requires. The placeholder registrar's guard asked `get()`, + * which answers for LOADED registrations only, so both keys read as free. The + * placeholder took them, the registry cleared the stubs under those keys, and + * every boot logged the registry's race warning once for each key. An authored + * `view:calendar` then drew the dashed placeholder until some other node + * happened to load the calendar chunk. + * + * The stubs here are declared with the console's arguments and land the way + * the two plugins register, `register(NAME, R, { namespace: 'view' })`. A chunk + * lands only when the test says so, so "before the chunk has loaded" and + * "after" are both observable, through the real registrar and the real + * `SchemaRenderer`. + * + * Both load orders are replayed. The console's boot order (stub still pending + * when the placeholders register) is the one that failed. The other order + * (chunk already loaded) is the control: the loaded-only guard always handled + * it, and the stub-aware guard must not change it. + */ +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { cleanup, render, within } from '@testing-library/react'; +import React from 'react'; +import { ComponentRegistry } from '@object-ui/core'; +import { SchemaRenderer } from '@object-ui/react'; +import type { BaseSchema } from '@object-ui/types'; +import { PlaceholderRenderer, registerPlaceholders } from '../renderers/placeholders'; + +/** + * The two node types this file's stand-in plugins register, declared to + * `@object-ui/types` the way an application declares a type it registers + * (objectui#11466): `SchemaRenderer`'s `schema` prop takes declared node types + * only. + */ +declare module '@object-ui/types' { + interface CustomNodeRegistry { + 'view:calendar': BaseSchema; + 'view:timeline': BaseSchema; + } +} + +/** The two protocol keys a console stub declares: [authored key, registered name]. */ +const CONTESTED: Array<[authored: 'view:calendar' | 'view:timeline', name: string]> = [ + ['view:calendar', 'calendar'], + ['view:timeline', 'timeline'], +]; + +/** The registry's own race detector, the warning objectui#11680 reported. */ +const RACE_WARNING = 'bare-name fallback is being overwritten'; + +function raceWarnings(warn: ReturnType): string[] { + return warn.mock.calls + .map((args: unknown[]) => (typeof args[0] === 'string' ? args[0] : '')) + .filter((text: string) => text.includes(RACE_WARNING)); +} + +/** + * Declare `name` the way the console does, behind a chunk that lands on + * command and then registers the plugin's renderer the way the plugin does. + */ +function declareGatedStub(name: string) { + let land!: () => void; + const gate = new Promise((resolve) => { + land = resolve; + }); + const Plugin = () =>
; + ComponentRegistry.registerLazy( + name, + async () => { + await gate; + ComponentRegistry.register(name, Plugin, { namespace: 'view', category: 'view' }); + }, + { namespace: 'view', category: 'view' }, + ); + return { land, Plugin }; +} + +afterEach(() => { + // Unmount first: an unregister notifies the registry's subscribers, and a + // renderer still mounted would re-render outside act(). + cleanup(); + // Every key either generation of the guard can leave behind, on both tables. + for (const [authored, name] of CONTESTED) { + ComponentRegistry.unregister(name, 'view'); + ComponentRegistry.unregister(name); + ComponentRegistry.unregister(authored, 'protocol-placeholder'); + ComponentRegistry.unregister(authored); + } +}); + +describe('a protocol placeholder never takes a key a pending lazy stub owns (objectui#11680)', () => { + it.each(CONTESTED)( + '%s stays with its pending stub when the placeholders register (the console boot order)', + async (authored, name) => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const { land, Plugin } = declareGatedStub(name); + registerPlaceholders(); + expect(raceWarnings(warn), 'registering the placeholders raced the stub').toEqual([]); + + // Before the chunk loads, the key is still the stub's. + expect(ComponentRegistry.get(authored)).not.toBe(PlaceholderRenderer); + expect(ComponentRegistry.hasLazy(authored)).toBe(true); + + // An authored node goes through the stub, which loads the plugin. + const before = render(); + expect(before.container.textContent).not.toContain('Component Placeholder'); + land(); + expect(await before.findByTestId(`plugin-${name}`)).toBeTruthy(); + + // After the chunk has loaded, the same renderer answers directly. + expect(ComponentRegistry.get(authored)).toBe(Plugin); + const after = render(); + expect(within(after.container).getByTestId(`plugin-${name}`)).toBeTruthy(); + + // Nor does the chunk landing: its registration names the stub's own type. + expect(raceWarnings(warn), 'the registry reported a race over the key').toEqual([]); + } finally { + warn.mockRestore(); + } + }, + ); + + it.each(CONTESTED)( + '%s keeps the plugin when its chunk loaded before the placeholders registered (control)', + async (authored, name) => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + const { land, Plugin } = declareGatedStub(name); + land(); + await ComponentRegistry.loadLazy(authored); + expect(ComponentRegistry.get(authored)).toBe(Plugin); + + registerPlaceholders(); + + expect(ComponentRegistry.get(authored)).toBe(Plugin); + const view = render(); + expect(within(view.container).getByTestId(`plugin-${name}`)).toBeTruthy(); + expect(raceWarnings(warn), 'the registry reported a race over the key').toEqual([]); + } finally { + warn.mockRestore(); + } + }, + ); + + it('still registers the placeholder for a protocol key nothing owns (control)', () => { + // `view:kanban` has no stub and no renderer in this file, so the guard must + // let the placeholder in: owning nothing is not the same as owned. + registerPlaceholders(); + expect(ComponentRegistry.get('view:kanban')).toBe(PlaceholderRenderer); + }); +}); diff --git a/packages/components/src/renderers/placeholders.tsx b/packages/components/src/renderers/placeholders.tsx index 4c56f29214..202f3062ae 100644 --- a/packages/components/src/renderers/placeholders.tsx +++ b/packages/components/src/renderers/placeholders.tsx @@ -137,8 +137,28 @@ export const PALETTE_PLACEHOLDER_BLOCKS = [ 'nav:menu', 'nav:breadcrumb', 'global:search', 'ai:suggestion', ]; -/** Register one placeholder, never overwriting a real implementation. */ +/** + * Register one placeholder, never over a key something else already owns: a + * loaded registration, or a pending `registerLazy` stub (objectui#11680). + * + * ⭐ The stub half is the one that was missing. `get()` answers for LOADED + * registrations only, so a key held by a stub whose chunk has not loaded read + * as free. The console declares `view:calendar` and `view:timeline` that way, + * and runs {@link registerPlaceholders} after its stubs. So the placeholder took + * both keys and the registry cleared the stubs under them, as it does for any + * key a registration takes. An authored `view:calendar` then drew the dashed + * scaffold until some other node happened to load the calendar chunk, and the + * registry's race warning fired for both keys on every boot. `hasLazy()` counts + * the pending stub, so the key stays with the plugin that declared it and + * `SchemaRenderer`'s lazy branch loads that plugin on first use. + * + * ⚠️ The check is made when the placeholder registers, so it only sees stubs + * declared before it. A stub declared afterwards finds the key already taken. + * That is why a host calls {@link registerPlaceholders} after its own stubs, as + * the console's `main.tsx` says it must. + */ function registerPlaceholder(type: string) { + if (ComponentRegistry.hasLazy(type)) return; if (!ComponentRegistry.get(type)) { ComponentRegistry.register(type, PlaceholderRenderer, { namespace: 'protocol-placeholder' }); } diff --git a/packages/core/src/registry/Registry.ts b/packages/core/src/registry/Registry.ts index 69457e1f5b..c7d525e29b 100644 --- a/packages/core/src/registry/Registry.ts +++ b/packages/core/src/registry/Registry.ts @@ -375,23 +375,28 @@ export type LazyComponentLoader = () => Promise; type LazyEntry = { loader: LazyComponentLoader; meta?: ComponentMeta; + /** + * The full type this stub DECLARES — `namespace:type`, or the bare type with + * no namespace. Written once, by `registerLazy`, on the line that computes the + * key the entry is stored under, and read by every collision guard that asks + * who a stub belongs to (objectui#11680). + * + * ⭐ Recorded rather than recomputed, because the entry is stored under TWO + * keys, its full type and its bare one, and a lookup cannot tell which of + * the two it hit. The helper this field replaces re-derived the claim as + * "the entry's namespace, prefixed onto the key it was looked up under". + * That is right for the bare key and doubles the namespace for the full key: + * the protocol placeholder for `view:calendar` claims the bare key + * `view:calendar`, finds the console's stub under its FULL key, and the race + * warning named the stub's claim as `view:view:calendar`, a type nothing + * declares. The ownership comparisons in `register`, `registerLazy` and + * `unregister` read the same doubled spelling. + */ + fullType: string; /** Pending import promise — reused when multiple consumers race. */ pending?: Promise; }; -/** - * The full type a pending lazy stub DECLARES for a bare key. - * - * `registerLazy` computes `namespace:type` and stores the stub under both that - * key and the bare one; nothing on the entry itself records which bare key it - * claims, so the claim has to be recomputed from the entry's own meta. Spelled - * once here because both doors' collision guards need it and a second copy - * would be a second thing to keep in step with `registerLazy`'s own line. - */ -function lazyStubFullType(type: string, entry: LazyEntry): string { - return entry.meta?.namespace ? `${entry.meta.namespace}:${type}` : type; -} - /** * Emit the spec's `dataSource` input for a registration whose renderer wraps * `ElementDataSourceGate` (objectui#6678). @@ -564,8 +569,7 @@ export class Registry { // owner and the opt-out settles this contest exactly as it does on the // lazy door. Both doors therefore prescribe the same thing now, which // `each door prescribes the remedy that is true for it` pins. - const stub = this.lazyEntries.get(type); - const stubType = stub ? lazyStubFullType(type, stub) : undefined; + const stubType = this.lazyEntries.get(type)?.fullType; if (stubType && stubType !== fullType) { console.warn( `Component "${type}" bare-name fallback is being overwritten by "${fullType}", ` + @@ -646,7 +650,7 @@ export class Registry { // The same ownership test, on the other table. if (namespace) { const bareStub = this.lazyEntries.get(type); - if (bareStub && lazyStubFullType(type, bareStub) === fullType) { + if (bareStub && bareStub.fullType === fullType) { this.lazyEntries.delete(type); } } @@ -668,7 +672,7 @@ export class Registry { */ registerLazy(type: string, loader: LazyComponentLoader, meta?: ComponentMeta) { const fullType = meta?.namespace ? `${meta.namespace}:${type}` : type; - const entry: LazyEntry = { loader, meta }; + const entry: LazyEntry = { loader, meta, fullType }; this.lazyEntries.set(fullType, entry); if (meta?.namespace && !meta?.skipFallback) { // Collision guard (objectui#9821). This door took the same bare-name @@ -698,11 +702,7 @@ export class Registry { // genuine contest. const loaded = this.components.get(type); const priorStub = loaded ? undefined : this.lazyEntries.get(type); - const claimedBy = loaded - ? loaded.type - : priorStub - ? lazyStubFullType(type, priorStub) - : undefined; + const claimedBy = loaded ? loaded.type : priorStub?.fullType; if (claimedBy && claimedBy !== fullType) { console.warn( `Lazy component "${type}" bare-name fallback is being overwritten by "${fullType}", ` + diff --git a/packages/core/src/registry/__tests__/Registry.test.ts b/packages/core/src/registry/__tests__/Registry.test.ts index 70076621bd..81853c1df4 100644 --- a/packages/core/src/registry/__tests__/Registry.test.ts +++ b/packages/core/src/registry/__tests__/Registry.test.ts @@ -700,4 +700,80 @@ describe('Registry', () => { expect(registry.hasLazy('dashboard', 'plugin-dashboard')).toBe(true); }); }); + + /** + * A stub reached under its FULL key is named by that full type, once + * (objectui#11680). + * + * `registerLazy` stores one entry under two keys, `namespace:type` and the + * bare `type`. A registration whose bare key is itself colon-shaped can land + * on the FULL one: the protocol placeholder for `view:calendar` claims the + * bare key `view:calendar`, which is where the console's + * `registerLazy('calendar', …, { namespace: 'view' })` stub lives. The guards + * used to re-derive the stub's claim by prefixing its namespace onto the key + * they looked it up under, which read `view:view:calendar` there, a type + * nothing declares. The stub now records its full type when it is declared, + * and each of the three readers below gets that one spelling. + * + * The registration shapes are written directly against a fresh registry, so + * these pins reach the registry's own doors. `@object-ui/components`' + * placeholder registrar no longer makes the first call at all while a stub is + * pending; `placeholder-lazy-stub-ownership-11680.test.tsx` pins that half. + */ + describe('a stub found under its FULL key names that full type once (objectui#11680)', () => { + const loader = () => Promise.resolve(); + const collisionWarnings = () => + consoleWarnSpy.mock.calls + .map((args: unknown[]) => (typeof args[0] === 'string' ? args[0] : '')) + .filter((text: string) => text.includes('bare-name fallback is being overwritten')); + /** The full type a collision warning says the key is already claimed for. */ + const claimantNamed = (text: string) => /already claims for "([^"]+)"/.exec(text)?.[1]; + + beforeEach(() => { + // The console's own declaration of the calendar view. + registry.registerLazy('calendar', loader, { namespace: 'view', category: 'view' }); + consoleWarnSpy.mockClear(); + }); + + it('the eager door names the stub by the full type it declared', () => { + registry.register('view:calendar', () => 'placeholder', { namespace: 'protocol-placeholder' }); + + const warned = collisionWarnings(); + expect(warned).toHaveLength(1); + expect(claimantNamed(warned[0])).toBe('view:calendar'); + expect(warned[0]).not.toContain('view:view:'); + }); + + it('the eager door still reports a registration whose own full type is the doubled spelling', () => { + // `view:` written twice: the registration's full type is + // `view:view:calendar`, exactly the doubled spelling. Compared against the + // doubled claim, the guard read the stub as this registration's own and + // said nothing while the bare `view:calendar` key changed hands. + registry.register('view:calendar', () => 'twice', { namespace: 'view' }); + + const warned = collisionWarnings(); + expect(warned).toHaveLength(1); + expect(claimantNamed(warned[0])).toBe('view:calendar'); + }); + + it('the lazy door names the prior stub by the full type it declared', () => { + registry.registerLazy('view:calendar', () => Promise.resolve(), { namespace: 'protocol-placeholder' }); + + const warned = collisionWarnings(); + expect(warned).toHaveLength(1); + expect(warned[0]).toContain('another pending stub'); + expect(claimantNamed(warned[0])).toBe('view:calendar'); + expect(warned[0]).not.toContain('view:view:'); + }); + + it('unregister of the doubled spelling leaves the stub it does not own', () => { + // Nothing was ever registered as `view:view:calendar`. Read through the + // doubled claim, the stub under bare `view:calendar` looked like that + // registration's, and this call deleted the console's calendar stub. + expect(registry.unregister('view:calendar', 'view')).toBe(false); + + expect(registry.hasLazy('calendar', 'view')).toBe(true); + expect(registry.hasLazy('calendar')).toBe(true); + }); + }); }); diff --git a/packages/plugin-report/src/__tests__/ReportRenderer.test.tsx b/packages/plugin-report/src/__tests__/ReportRenderer.test.tsx index 469001f476..56ec2ce026 100644 --- a/packages/plugin-report/src/__tests__/ReportRenderer.test.tsx +++ b/packages/plugin-report/src/__tests__/ReportRenderer.test.tsx @@ -23,6 +23,10 @@ vi.mock('@object-ui/core', async () => { ComponentRegistry: { get: vi.fn(), register: vi.fn(), + // `@object-ui/components`' placeholder registrar asks this before it + // registers, at module load (objectui#11680). No stub is pending + // in this suite, so every key answers false. + hasLazy: vi.fn(() => false), } }; });