Skip to content

Commit 02aef0c

Browse files
authored
fix(sdui): a kind:'html' page can use lazily-registered blocks, and recovers when one registers late (#2988)
#2953 had a twin one tier over, unreported. The whitelist a kind:'html' page's source compiles against was built from getAllTypes() + getConfig() — both loaded-only — so any block registered via registerLazy() was rejected as "not an allowed component". Worse blast radius than the react tier: a compile diagnostic fails the WHOLE page, so a single <object-kanban> replaced everything with "HTML page failed to compile (2)". And it never recovered — layoutElement was memoised on [schema, pageType] with no registry signal, so the cached error panel outlived the plugin landing. Measured with a probe, not inferred. ComponentRegistry gains three lazy-aware reads: getKnownTypes() (loaded plus pending stubs — the set a whitelist or manifest should be built from), getMeta() (metadata through to a stub), and getVersion() (monotonic counter of changes to the known set; the previous cache key was the type COUNT, which one registration plus one unregistration leaves untouched). getAllTypes() and getConfig() keep their loaded-only meaning and now document it. getJsxManifest() builds from those; PageRenderer subscribes to the registry so a page that could not compile retries when the registry grows. A stub has no `inputs` yet, so its props surface as unknown-prop warnings rather than errors — the page compiles and the inner SchemaRenderer swaps in the real block. The runtime whitelist being permissive and the build-time artifact being strict (assertFullyLoaded, #2979) is deliberate: authoring-time prop validation keeps its full inputs. Tests include a negative control — a tag nothing registered is still rejected, since that whitelist is what keeps a parsed page from rendering arbitrary tags.
1 parent 9e21004 commit 02aef0c

5 files changed

Lines changed: 316 additions & 11 deletions

File tree

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
---
2+
"@object-ui/core": minor
3+
"@object-ui/components": patch
4+
---
5+
6+
fix(sdui): a `kind:'html'` page can use lazily-registered blocks, and recovers when one registers late
7+
8+
objectui#2953 had a twin one tier over, unreported. The whitelist a
9+
`kind:'html'` page's source compiles against was built from `getAllTypes()` +
10+
`getConfig()` — both loaded-only — so any block registered via `registerLazy()`
11+
was rejected as *"not an allowed component"*.
12+
13+
The blast radius is worse than the react tier's. There, a missing block cost one
14+
identifier; here a compile diagnostic fails the **whole page**, so a single
15+
`<object-kanban>` replaced the entire page with `HTML page failed to compile (2)`.
16+
And it never recovered: `layoutElement` was memoised on `[schema, pageType]` with
17+
no registry signal, so the cached error panel outlived the plugin actually
18+
landing — permanently broken for the session.
19+
20+
`ComponentRegistry` gains three lazy-aware reads:
21+
22+
- `getKnownTypes()` — loaded registrations **plus** pending lazy stubs, deduped.
23+
The set a whitelist or manifest should be built from. `getAllTypes()` keeps its
24+
loaded-only meaning ("what can render right now") and now says so.
25+
- `getMeta(type, namespace?)` — metadata from the loaded registration, else from
26+
a pending stub. `getConfig()` stays loaded-only, since callers read
27+
`.component` off it.
28+
- `getVersion()` — monotonic counter of changes to the known set, bumped on
29+
register / unregister / registerLazy. A cache key that a type *count* cannot
30+
substitute for: one registration plus one unregistration leaves the count
31+
untouched while the set changed.
32+
33+
`getJsxManifest()` builds from those, and `PageRenderer` subscribes to the
34+
registry so a page that could not compile retries when the registry grows.
35+
36+
A stub carries no `inputs` yet, so its props surface as `unknown-prop` warnings
37+
rather than errors — the page compiles and renders, and the inner
38+
`SchemaRenderer` triggers the loader and swaps in the real block. Authoring-time
39+
prop validation is unaffected: `sdui.manifest.json` is generated with every
40+
plugin eagerly loaded, and asserts as much.
Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*
8+
* A `kind:'html'` page can use lazily-registered blocks (objectui#2953, html tier).
9+
*
10+
* Same root cause as the react tier, worse blast radius. The whitelist a page's
11+
* source is compiled against was built from `getAllTypes()` + `getConfig()` —
12+
* both loaded-only — so a block registered via `registerLazy()` was rejected as
13+
* "not an allowed component". And because a compile diagnostic fails the WHOLE
14+
* page rather than the one node, `<object-kanban>` anywhere in the source
15+
* replaced the entire page with an error panel. Load-order dependent, so the
16+
* same page worked or didn't depending on what else had rendered first.
17+
*
18+
* It also never recovered: `layoutElement` was memoised on `[schema, pageType]`
19+
* with no registry signal, so the cached error panel outlived the plugin
20+
* actually landing.
21+
*/
22+
23+
import { describe, it, expect, beforeAll, afterEach } from 'vitest';
24+
import { render, waitFor } from '@testing-library/react';
25+
import React from 'react';
26+
import { ComponentRegistry } from '@object-ui/core';
27+
import { SchemaRenderer, AdapterCtx } from '@object-ui/react';
28+
29+
const adapter = { find: async () => [] } as any;
30+
31+
/** Registers a lazy `object-kanban` whose chunk lands only when you say so. */
32+
function lazyKanban() {
33+
let land!: () => void;
34+
const gate = new Promise<void>((resolve) => {
35+
land = resolve;
36+
});
37+
ComponentRegistry.registerLazy(
38+
'object-kanban',
39+
async () => {
40+
await gate;
41+
ComponentRegistry.register('object-kanban', () => <div data-testid="kanban-double" />, {
42+
namespace: 'plugin-kanban',
43+
});
44+
},
45+
{ namespace: 'plugin-kanban', category: 'view' },
46+
);
47+
return land;
48+
}
49+
50+
function renderHtmlPage(source: string) {
51+
return render(
52+
<AdapterCtx.Provider value={adapter}>
53+
<SchemaRenderer schema={{ type: 'home', kind: 'html', name: 'test_page', source }} />
54+
</AdapterCtx.Provider>,
55+
);
56+
}
57+
58+
beforeAll(async () => {
59+
// Registers PageRenderer for `type:'home'`, which dispatches kind:'html'.
60+
await import('../renderers');
61+
}, 30000);
62+
63+
afterEach(() => {
64+
ComponentRegistry.unregister('object-kanban', 'plugin-kanban');
65+
ComponentRegistry.unregister('object-kanban');
66+
});
67+
68+
describe('kind:\'html\' page with a lazily-registered block', () => {
69+
it('compiles instead of failing the whole page, and renders the block once loaded', async () => {
70+
const land = lazyKanban();
71+
72+
const { container, findByTestId } = renderHtmlPage(
73+
`<flex><object-kanban object="showcase_project" /></flex>`,
74+
);
75+
76+
// Before: 'HTML page failed to compile (2)' — `<object-kanban> is not an
77+
// allowed component` + `is not a known component` — and no <flex> either.
78+
expect(container.textContent).not.toContain('failed to compile');
79+
80+
land();
81+
expect(await findByTestId('kanban-double')).toBeTruthy();
82+
});
83+
84+
it('recovers a page that compiled before the block was registered', async () => {
85+
// Nothing knows `object-kanban` yet, so this page genuinely cannot compile.
86+
const { container } = renderHtmlPage(`<flex><object-kanban object="showcase_project" /></flex>`);
87+
expect(container.textContent).toContain('failed to compile');
88+
89+
// The plugin registers late — a legitimate case (a plugin installed at
90+
// runtime, or any registration that races first paint).
91+
ComponentRegistry.register('object-kanban', () => <div data-testid="kanban-double" />, {
92+
namespace: 'plugin-kanban',
93+
});
94+
95+
// Before: the memoised error panel outlived the registration, permanently.
96+
await waitFor(() => expect(container.querySelector('[data-testid=kanban-double]')).toBeTruthy());
97+
expect(container.textContent).not.toContain('failed to compile');
98+
});
99+
100+
it('still rejects a tag nothing has registered at all', async () => {
101+
const { container } = renderHtmlPage(`<flex><totally-not-a-block /></flex>`);
102+
103+
// The negative control. Admitting lazy stubs must not turn the whitelist
104+
// into "anything goes" — that whitelist is what keeps a parsed page from
105+
// rendering arbitrary tags.
106+
await waitFor(() => expect(container.textContent).toContain('failed to compile'));
107+
expect(container.textContent).toContain('totally-not-a-block');
108+
});
109+
});

packages/components/src/renderers/layout/page.tsx

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -366,16 +366,26 @@ function getJsxManifest() {
366366
// config's `.type` is always the namespaced form (Registry stores the bare
367367
// alias pointing at the namespaced type), so getAllConfigs() alone would
368368
// never admit the bare `<flex>` tag authors write.
369-
const types = ComponentRegistry.getAllTypes();
370-
if (_jsxManifest === null || _jsxManifestSig !== types.length) {
371-
const configs = types.map((t) => {
372-
const cfg = ComponentRegistry.getConfig(t) as
373-
| { namespace?: string; isContainer?: boolean; inputs?: unknown }
374-
| undefined;
375-
return { type: t, namespace: cfg?.namespace, isContainer: cfg?.isContainer, inputs: cfg?.inputs };
369+
//
370+
// KNOWN types, not loaded ones. A lazily-registered block is a real member of
371+
// this app's vocabulary; leaving it out of the whitelist rejected
372+
// `<object-kanban>` as "not an allowed component" and — because a compile
373+
// error fails the WHOLE page, not just that node — took the entire page down
374+
// with it, load-order dependently (objectui#2953, html tier).
375+
//
376+
// A stub has no `inputs` yet, so its props come back as `unknown-prop`
377+
// WARNINGS rather than errors: the page compiles and renders, and the inner
378+
// SchemaRenderer triggers the loader and swaps in the real block. Authoring
379+
// time still gets full prop validation — `sdui.manifest.json` is generated
380+
// with every plugin eagerly loaded, and asserts as much.
381+
const version = ComponentRegistry.getVersion();
382+
if (_jsxManifest === null || _jsxManifestSig !== version) {
383+
const configs = ComponentRegistry.getKnownTypes().map((t) => {
384+
const meta = ComponentRegistry.getMeta(t);
385+
return { type: t, namespace: meta?.namespace, isContainer: meta?.isContainer, inputs: meta?.inputs };
376386
});
377387
_jsxManifest = manifestFromConfigs(configs as unknown as Parameters<typeof manifestFromConfigs>[0]);
378-
_jsxManifestSig = types.length;
388+
_jsxManifestSig = version;
379389
}
380390
return _jsxManifest;
381391
}
@@ -389,6 +399,13 @@ export const PageRenderer: React.FC<{
389399
[key: string]: any;
390400
}> = ({ schema, className, ...props }) => {
391401
const pageType = schema.pageType || 'record';
402+
// A `kind:'html'` page compiles its source against the registry, and a
403+
// compile error fails the whole page. Without this the failure is permanent
404+
// for the session: `layoutElement` is memoised, so a page that compiled
405+
// before the block it needs was registered kept rendering the cached error
406+
// panel even after the plugin landed. Mirrors SchemaRenderer's subscription.
407+
const [registryTick, bumpRegistryTick] = React.useReducer((n: number) => n + 1, 0);
408+
React.useEffect(() => ComponentRegistry.subscribe(bumpRegistryTick), []);
392409
// Spec PageSchema declares `label` (required); `title` is the objectui
393410
// spelling. Dual-read so a spec-authored page still renders its header
394411
// (framework#1878 §3 naming-drift recheck).
@@ -480,7 +497,12 @@ export const PageRenderer: React.FC<{
480497
default:
481498
return <RecordPageLayout schema={schema} />;
482499
}
483-
}, [schema, pageType]);
500+
// `registryTick` only matters to the kind:'html' branch above, which
501+
// recompiles against the (now larger) whitelist. Every other branch just
502+
// rebuilds the same element type with the same props, which React
503+
// reconciles in place — no remount, no state loss (pinned by
504+
// react-page-state.test.tsx).
505+
}, [schema, pageType, registryTick]);
484506

485507
// Full-bleed: a page whose `main` region is declared `width: 'full'` fills
486508
// the viewport with NO centered max-width cap — for dashboards / 大屏 /

packages/core/src/registry/Registry.ts

Lines changed: 59 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,14 @@ export class Registry<T = any> {
108108
* load completes.
109109
*/
110110
private listeners = new Set<() => void>();
111+
/**
112+
* Bumped whenever the set of KNOWN types changes — a registration, an
113+
* unregistration, or a new lazy stub. A cheap cache key for consumers that
114+
* derive something from the whole registry (whitelists, manifests, palettes)
115+
* and need to rebuild when it grows. Counting types is not a substitute: a
116+
* registration paired with an unregistration leaves the count untouched.
117+
*/
118+
private version = 0;
111119

112120
/**
113121
* Register a component with optional namespace support.
@@ -230,6 +238,10 @@ export class Registry<T = any> {
230238
if (meta?.namespace && !meta?.skipFallback) {
231239
this.lazyEntries.set(type, entry);
232240
}
241+
// Bump the version but do NOT notify: the set of KNOWN types grew, so
242+
// whitelists and manifests must rebuild, but nothing new can render yet —
243+
// waking every subscriber for each stub at boot would be pure churn.
244+
this.version++;
233245
}
234246

235247
/**
@@ -274,6 +286,7 @@ export class Registry<T = any> {
274286
}
275287

276288
private notify() {
289+
this.version++;
277290
for (const listener of this.listeners) {
278291
try {
279292
listener();
@@ -283,6 +296,14 @@ export class Registry<T = any> {
283296
}
284297
}
285298

299+
/**
300+
* Monotonic counter of changes to the set of known types. Rebuild anything
301+
* derived from the whole registry when this moves — see {@link getKnownTypes}.
302+
*/
303+
getVersion(): number {
304+
return this.version;
305+
}
306+
286307
/**
287308
* Get a component by type. Supports both namespaced and non-namespaced lookups.
288309
*
@@ -356,14 +377,50 @@ export class Registry<T = any> {
356377
}
357378

358379
/**
359-
* Get all registered component types.
360-
*
380+
* Get all LOADED component types — what can be rendered right now.
381+
*
382+
* Pending `registerLazy` stubs are not included; use {@link getKnownTypes}
383+
* for the question "is this a type this app knows about", which is what a
384+
* whitelist or manifest wants (objectui#2953).
385+
*
361386
* @returns Array of all component type identifiers
362387
*/
363388
getAllTypes(): string[] {
364389
return Array.from(this.components.keys());
365390
}
366391

392+
/**
393+
* Every type the registry can resolve — loaded registrations PLUS pending
394+
* lazy stubs, deduped. Both the bare and namespaced keys are included, as
395+
* they are in {@link getAllTypes}.
396+
*
397+
* This is the set to build a whitelist or a manifest from. Keying one off
398+
* `getAllTypes()` instead makes it depend on which plugin chunks happen to
399+
* have loaded: a lazily-registered block gets rejected as unknown, and
400+
* whether it does varies by session (objectui#2953).
401+
*/
402+
getKnownTypes(): string[] {
403+
return Array.from(new Set([...this.components.keys(), ...this.lazyEntries.keys()]));
404+
}
405+
406+
/**
407+
* Metadata for `type` — from the loaded registration when there is one,
408+
* otherwise from a pending `registerLazy` stub.
409+
*
410+
* Use this when you need to know what a type IS (namespace, container-ness,
411+
* declared inputs). {@link getConfig} answers a different question — "can I
412+
* render this right now" — and is deliberately loaded-only, since callers
413+
* read `.component` off it.
414+
*
415+
* A stub carries only what `registerLazy` was given, so `inputs` is usually
416+
* absent until the chunk loads. Consumers should treat that as "not yet
417+
* known", not as "declares no props".
418+
*/
419+
getMeta(type: string, namespace?: string): ComponentMeta | undefined {
420+
const key = namespace ? `${namespace}:${type}` : type;
421+
return this.components.get(key) ?? this.lazyEntries.get(key)?.meta;
422+
}
423+
367424
/**
368425
* Get all registered component configurations.
369426
*

packages/core/src/registry/__tests__/Registry.test.ts

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,83 @@ describe('Registry', () => {
173173
});
174174
});
175175

176+
describe('getKnownTypes() / getMeta() / getVersion() — lazy-aware reads (objectui#2953)', () => {
177+
const loader = () => Promise.resolve();
178+
179+
it('getKnownTypes() includes pending lazy stubs; getAllTypes() does not', () => {
180+
registry.register('button', () => 'b1');
181+
registry.registerLazy('object-kanban', loader, { namespace: 'plugin-kanban' });
182+
183+
// getAllTypes() answers "what can render right now" — the stub cannot.
184+
expect(registry.getAllTypes()).not.toContain('object-kanban');
185+
// getKnownTypes() answers "what does this app know about" — the question
186+
// a whitelist or a manifest is actually asking. Building one off
187+
// getAllTypes() rejected lazily-registered blocks as unknown.
188+
expect(registry.getKnownTypes()).toContain('object-kanban');
189+
expect(registry.getKnownTypes()).toContain('plugin-kanban:object-kanban');
190+
expect(registry.getKnownTypes()).toContain('button');
191+
});
192+
193+
it('getKnownTypes() dedupes a type that is both loaded and stubbed', () => {
194+
registry.registerLazy('object-map', loader, { namespace: 'plugin-map' });
195+
registry.register('object-map', () => 'm', { namespace: 'plugin-map' });
196+
197+
const known = registry.getKnownTypes();
198+
expect(known.filter((t) => t === 'object-map')).toHaveLength(1);
199+
});
200+
201+
it('getMeta() reads through to a stub, getConfig() stays loaded-only', () => {
202+
registry.registerLazy('object-gantt', loader, {
203+
namespace: 'plugin-gantt',
204+
category: 'view',
205+
isContainer: false,
206+
});
207+
208+
expect(registry.getConfig('object-gantt')).toBeUndefined();
209+
expect(registry.getMeta('object-gantt')).toMatchObject({
210+
namespace: 'plugin-gantt',
211+
category: 'view',
212+
isContainer: false,
213+
});
214+
// A stub has no declared inputs until its chunk lands — "not yet known",
215+
// which consumers must not read as "declares no props".
216+
expect(registry.getMeta('object-gantt')?.inputs).toBeUndefined();
217+
});
218+
219+
it('getMeta() prefers the loaded registration once the chunk lands', () => {
220+
registry.registerLazy('object-chart', loader, { namespace: 'plugin-charts' });
221+
registry.register('object-chart', () => 'c', {
222+
namespace: 'plugin-charts',
223+
inputs: [{ name: 'type', type: 'string' }],
224+
});
225+
226+
expect(registry.getMeta('object-chart')?.inputs).toHaveLength(1);
227+
});
228+
229+
it('getMeta() honours an explicit namespace', () => {
230+
registry.registerLazy('kanban', loader, { namespace: 'view' });
231+
232+
expect(registry.getMeta('kanban', 'view')).toBeTruthy();
233+
expect(registry.getMeta('kanban', 'nope')).toBeUndefined();
234+
});
235+
236+
it('getVersion() moves on every change to the known set', () => {
237+
const start = registry.getVersion();
238+
registry.register('button', () => 'b1');
239+
const afterRegister = registry.getVersion();
240+
registry.registerLazy('object-map', loader, { namespace: 'plugin-map' });
241+
const afterLazy = registry.getVersion();
242+
registry.unregister('button');
243+
const afterUnregister = registry.getVersion();
244+
245+
// Counting types cannot substitute for this: a registration paired with
246+
// an unregistration leaves the count untouched while the set changed.
247+
expect(afterRegister).toBeGreaterThan(start);
248+
expect(afterLazy).toBeGreaterThan(afterRegister);
249+
expect(afterUnregister).toBeGreaterThan(afterLazy);
250+
});
251+
});
252+
176253
describe('getAllTypes() and getAllConfigs()', () => {
177254
it('should return all registered types including namespaced ones', () => {
178255
registry.register('button', () => 'b1');

0 commit comments

Comments
 (0)