Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .changeset/9821-registerlazy-cross-table-collision-warning.md
Original file line number Diff line number Diff line change
@@ -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.
83 changes: 83 additions & 0 deletions packages/core/src/registry/Registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,19 @@ type LazyEntry = {
pending?: Promise<unknown>;
};

/**
* 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).
Expand Down Expand Up @@ -502,6 +515,34 @@ export class Registry<T = any> {
`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
Expand Down Expand Up @@ -558,6 +599,48 @@ export class Registry<T = any> {
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
Expand Down
128 changes: 128 additions & 0 deletions packages/core/src/registry/__tests__/Registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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/);
});
});
});
Loading