diff --git a/.changeset/9821-registerlazy-cross-table-collision-warning.md b/.changeset/9821-registerlazy-cross-table-collision-warning.md new file mode 100644 index 0000000000..23dee0c7bc --- /dev/null +++ b/.changeset/9821-registerlazy-cross-table-collision-warning.md @@ -0,0 +1,13 @@ +--- +'@object-ui/core': patch +--- + +`Registry.registerLazy` now reports a bare-name collision, and `Registry.register`'s existing report is no longer blind to the other table. + +Clause-②: no — this adds no exported symbol, no key on a published payload, and relaxes no accept set. Which registrations the registry ACCEPTS is unchanged, and so is what `skipFallback` does; only what the registry SAYS changes. + +The registry has two doors onto one bare key: `register` writes `components`, `registerLazy` writes `lazyEntries`. `registerLazy` took the same `namespace && !skipFallback` fallback branch with no collision check at all, and `register`'s check read only its own table — so a contest that spans the two tables was reported by neither. Replaying this repository's own 425 declared registration claims against a real registry emitted zero collision warnings in every order, including for the one genuinely contested bare key. + +Both doors now consult both tables and key on the declared full type, so a contest is reported whichever order the declarations arrive in. The ordinary stub-then-real lifecycle stays silent: a stub and the registration that satisfies it name one full type, and so do the bare keys this repository stubs twice from two files with different loader closures. + +The two doors give different advice on purpose. `registerLazy` names `skipFallback: true`, which settles it there. `register` does not, because it clears the bare stub whether or not the fallback is taken, so that opt-out would leave the bare key resolving to nothing; it asks for the two declarations to agree on one full type instead. diff --git a/packages/core/src/registry/Registry.ts b/packages/core/src/registry/Registry.ts index 794aefc00f..89417a7382 100644 --- a/packages/core/src/registry/Registry.ts +++ b/packages/core/src/registry/Registry.ts @@ -356,6 +356,19 @@ type LazyEntry = { 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). @@ -502,6 +515,34 @@ export class Registry { `If this is intentional keep going; otherwise register "${fullType}" with ` + `{ skipFallback: true } so it doesn't claim the bare "${type}" key.`, ); + } else if (!existing) { + // The CROSS-TABLE half of the same guard (objectui#9821), and the half + // the contest objectui#9533 measured actually went through. The check + // above reads `components` only, so a bare key held by a pending STUB + // is invisible to it: the console declared bare `dashboard` for + // `plugin-dashboard:dashboard` at boot, this door took the same bare key + // for `view:dashboard` when the chunk landed, and because `components` + // still held nothing under `dashboard` at that moment, NOTHING warned. + // Measured, not reasoned: replaying that card's three real claimants + // against a real registry emitted zero warnings in BOTH orders. + // + // ⚠️ The guard has to fire HERE and not only in `registerLazy`, because + // a contest that is only reported in one registration order is reported + // in the order this repository does not boot in. The `report-` and + // `timeline-bare-key-ownership` pins replay both orders for exactly + // that reason. + const stub = this.lazyEntries.get(type); + const stubType = stub ? lazyStubFullType(type, stub) : undefined; + if (stubType && stubType !== fullType) { + console.warn( + `Component "${type}" bare-name fallback is being overwritten by "${fullType}", ` + + `which a pending lazy stub already claims for "${stubType}". Which one an ` + + `authored "${type}" node resolves to depends on whether that chunk has loaded. ` + + `Make the two declarations name ONE full type — { skipFallback: true } does not ` + + `settle this one, because registering "${fullType}" clears the bare "${type}" ` + + `stub either way.`, + ); + } } this.components.set(type, { type: fullType, // Keep reference to namespaced type @@ -558,6 +599,48 @@ export class Registry { const entry: LazyEntry = { loader, meta }; this.lazyEntries.set(fullType, entry); if (meta?.namespace && !meta?.skipFallback) { + // Collision guard (objectui#9821). This door took the same bare-name + // fallback branch as `register` with no check at all, so a stub could + // take the bare key off another declaration in total silence — and the + // silence is why the class stayed invisible: two of the three claimants + // in the contest objectui#9533 filed came in through here. + // + // ⭐ The predicate keys on the FULL TYPE and reads BOTH tables, and both + // halves of that were decided by the census rather than chosen: + // + // - BOTH tables, because the contention is cross-table. Copying the + // guard above verbatim would compare `lazyEntries` against itself, + // and the one contested bare key on this tree is a stub in this table + // against a loaded registration in the other one. A same-table check + // would not have caught the card that produced this one. + // - FULL TYPE and not loader identity, because the ordinary correct + // shape here is a stub re-declared with a DIFFERENT loader closure + // for the SAME full type: two console files drive the same plugin + // set, and nine of this tree's bare keys are registered twice that + // way. Keying on the loader would warn on all nine, every boot. + // + // Census taken on the declared population of this repository (the same + // one `check-registry-bare-name-collisions.mjs` reads): of 31 bare keys + // a stub claims, 30 name exactly one full type across both tables — the + // stub-then-real lifecycle this must stay silent about — and one is a + // 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; + if (claimedBy && claimedBy !== fullType) { + console.warn( + `Lazy component "${type}" bare-name fallback is being overwritten by "${fullType}", ` + + `which ${loaded ? 'a loaded registration' : 'another pending stub'} already claims ` + + `for "${claimedBy}". If this is intentional keep going; otherwise register ` + + `"${fullType}" with { skipFallback: true } so it doesn't claim the bare "${type}" ` + + `key — on this door that opt-out does settle it, because the stub then claims only ` + + `"${fullType}".`, + ); + } this.lazyEntries.set(type, entry); } // Bump the version but do NOT notify: the set of KNOWN types grew, so diff --git a/packages/core/src/registry/__tests__/Registry.test.ts b/packages/core/src/registry/__tests__/Registry.test.ts index 10df100bec..4867f2c154 100644 --- a/packages/core/src/registry/__tests__/Registry.test.ts +++ b/packages/core/src/registry/__tests__/Registry.test.ts @@ -462,4 +462,132 @@ describe('Registry', () => { expect(registry.get('textarea')).toBe(field); // bare clobbered (the regression) }); }); + + /** + * CROSS-TABLE bare-name collisions (objectui#9821). + * + * The registry has two doors onto one bare key: `register` writes + * `components`, `registerLazy` writes `lazyEntries`. Before this card only + * `register` checked for a collision, and it read only its own table — so + * the contest objectui#9533 filed, where a console stub claims bare + * `dashboard` for `plugin-dashboard:dashboard` and the package then claims + * the same bare key for `view:dashboard`, produced ZERO warnings. That zero + * was measured against a real registry, in BOTH orders, before this fix. + * + * ⭐ Both orders are asserted deliberately, the discipline + * `report-bare-key-ownership` / `timeline-bare-key-ownership` established: a + * contest reported in only one registration order is a detector whose answer + * depends on when it was asked, which is the defect, not the fix. + */ + describe('cross-table bare-name collisions (objectui#9821)', () => { + 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')); + + it('warns when a LOADED registration takes a bare key a pending stub claims (the objectui#9533 order)', () => { + // What the console actually does: stubs at boot, chunk later. + registry.registerLazy('dashboard', loader, { namespace: 'plugin-dashboard' }); + consoleWarnSpy.mockClear(); + registry.register('dashboard', () => 'view', { namespace: 'view' }); + + const warned = collisionWarnings(); + expect(warned).toHaveLength(1); + expect(warned[0]).toContain('view:dashboard'); + expect(warned[0]).toContain('plugin-dashboard:dashboard'); + expect(warned[0]).toContain('pending lazy stub'); + }); + + it('warns when a stub takes a bare key a LOADED registration claims (the reverse order)', () => { + registry.register('dashboard', () => 'view', { namespace: 'view' }); + consoleWarnSpy.mockClear(); + registry.registerLazy('dashboard', loader, { namespace: 'plugin-dashboard' }); + + const warned = collisionWarnings(); + expect(warned).toHaveLength(1); + expect(warned[0]).toContain('Lazy component "dashboard"'); + expect(warned[0]).toContain('plugin-dashboard:dashboard'); + expect(warned[0]).toContain('view:dashboard'); + }); + + it('warns when one stub takes a bare key another stub claims for a different full type', () => { + registry.registerLazy('dashboard', loader, { namespace: 'plugin-dashboard' }); + consoleWarnSpy.mockClear(); + registry.registerLazy('dashboard', () => Promise.resolve(), { namespace: 'view' }); + + const warned = collisionWarnings(); + expect(warned).toHaveLength(1); + expect(warned[0]).toContain('another pending stub'); + }); + + it('stays SILENT on the ordinary stub-then-real lifecycle, in both orders', () => { + // 30 of this tree's 31 stub-claimed bare keys are exactly this shape: the + // console declares `plugin-charts:chart` and the package registers the + // same full type. A guard that fired here would fire at every boot. + registry.registerLazy('chart', loader, { namespace: 'plugin-charts' }); + registry.register('chart', () => 'chart', { namespace: 'plugin-charts' }); + expect(collisionWarnings()).toHaveLength(0); + + const reverse = new Registry(); + reverse.register('chart', () => 'chart', { namespace: 'plugin-charts' }); + reverse.registerLazy('chart', loader, { namespace: 'plugin-charts' }); + expect(collisionWarnings()).toHaveLength(0); + }); + + it('stays SILENT when two files stub one full type with DIFFERENT loader closures', () => { + // `preview-gallery.tsx` and `register-plugins.ts` drive the same plugin + // set, so nine bare keys on this tree are stubbed twice with two distinct + // arrow functions. This is why the predicate keys on the full type and + // NOT on loader identity — the latter would warn on all nine, every boot. + registry.registerLazy('metric', () => Promise.resolve(), { namespace: 'plugin-dashboard' }); + registry.registerLazy('metric', () => Promise.resolve(), { namespace: 'plugin-dashboard' }); + + expect(collisionWarnings()).toHaveLength(0); + }); + + it('stays SILENT when the stub declines the bare key with skipFallback', () => { + registry.register('dashboard', () => 'view', { namespace: 'view' }); + consoleWarnSpy.mockClear(); + registry.registerLazy('dashboard', loader, { namespace: 'plugin-dashboard', skipFallback: true }); + + expect(collisionWarnings()).toHaveLength(0); + expect(registry.hasLazy('dashboard', 'plugin-dashboard')).toBe(true); + expect(registry.hasLazy('dashboard')).toBe(false); // bare key left alone + }); + + /** + * ⭐ Why the two doors give DIFFERENT advice, pinned as behaviour and not + * only as prose. `register` clears the bare stub unconditionally — outside + * the fallback branch — so telling that door's author to pass + * `skipFallback: true` would leave the bare key resolving to nothing at + * all. On the lazy door the same opt-out really does settle it. + */ + it('skipFallback on the EAGER door does not preserve the stub\'s bare claim', () => { + registry.registerLazy('dashboard', loader, { namespace: 'plugin-dashboard' }); + expect(registry.hasLazy('dashboard')).toBe(true); + + registry.register('dashboard', () => 'view', { namespace: 'view', skipFallback: true }); + + // The bare key is now claimed by nobody: the stub was deleted anyway and + // the registration declined to take it. + expect(registry.hasLazy('dashboard')).toBe(false); + expect(registry.get('dashboard')).toBeUndefined(); + }); + + it('each door prescribes the remedy that is true for it', () => { + registry.register('dashboard', () => 'view', { namespace: 'view' }); + consoleWarnSpy.mockClear(); + registry.registerLazy('dashboard', loader, { namespace: 'plugin-dashboard' }); + expect(collisionWarnings()[0]).toMatch(/skipFallback: true/); + + const other = new Registry(); + consoleWarnSpy.mockClear(); + other.registerLazy('dashboard', loader, { namespace: 'plugin-dashboard' }); + other.register('dashboard', () => 'view', { namespace: 'view' }); + // ⛔ Not a wording preference: the eager door must not prescribe an + // opt-out that the test above shows does not settle the contest. + expect(collisionWarnings()[0]).toMatch(/does not\s+settle this one/); + }); + }); });