diff --git a/.changeset/html-page-lazy-blocks.md b/.changeset/html-page-lazy-blocks.md new file mode 100644 index 000000000..8d1905ed8 --- /dev/null +++ b/.changeset/html-page-lazy-blocks.md @@ -0,0 +1,40 @@ +--- +"@object-ui/core": minor +"@object-ui/components": patch +--- + +fix(sdui): a `kind:'html'` page can use lazily-registered blocks, and recovers when one registers late + +objectui#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"*. + +The blast radius is worse than the react tier's. There, a missing block cost one +identifier; here a compile diagnostic fails the **whole page**, so a single +`` replaced the entire page 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 actually +landing — permanently broken for the session. + +`ComponentRegistry` gains three lazy-aware reads: + +- `getKnownTypes()` — loaded registrations **plus** pending lazy stubs, deduped. + The set a whitelist or manifest should be built from. `getAllTypes()` keeps its + loaded-only meaning ("what can render right now") and now says so. +- `getMeta(type, namespace?)` — metadata from the loaded registration, else from + a pending stub. `getConfig()` stays loaded-only, since callers read + `.component` off it. +- `getVersion()` — monotonic counter of changes to the known set, bumped on + register / unregister / registerLazy. A cache key that a type *count* cannot + substitute for: one registration plus one unregistration leaves the count + untouched while the set changed. + +`getJsxManifest()` builds from those, and `PageRenderer` subscribes to the +registry so a page that could not compile retries when the registry grows. + +A stub carries no `inputs` yet, so its props surface as `unknown-prop` warnings +rather than errors — the page compiles and renders, and the inner +`SchemaRenderer` triggers the loader and swaps in the real block. Authoring-time +prop validation is unaffected: `sdui.manifest.json` is generated with every +plugin eagerly loaded, and asserts as much. diff --git a/packages/components/src/__tests__/html-page-lazy-blocks.test.tsx b/packages/components/src/__tests__/html-page-lazy-blocks.test.tsx new file mode 100644 index 000000000..d6a6d365f --- /dev/null +++ b/packages/components/src/__tests__/html-page-lazy-blocks.test.tsx @@ -0,0 +1,109 @@ +/** + * 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 `kind:'html'` page can use lazily-registered blocks (objectui#2953, html tier). + * + * Same root cause as the react tier, worse blast radius. The whitelist a page's + * source is compiled against was built from `getAllTypes()` + `getConfig()` — + * both loaded-only — so a block registered via `registerLazy()` was rejected as + * "not an allowed component". And because a compile diagnostic fails the WHOLE + * page rather than the one node, `` anywhere in the source + * replaced the entire page with an error panel. Load-order dependent, so the + * same page worked or didn't depending on what else had rendered first. + * + * It also never recovered: `layoutElement` was memoised on `[schema, pageType]` + * with no registry signal, so the cached error panel outlived the plugin + * actually landing. + */ + +import { describe, it, expect, beforeAll, afterEach } from 'vitest'; +import { render, waitFor } from '@testing-library/react'; +import React from 'react'; +import { ComponentRegistry } from '@object-ui/core'; +import { SchemaRenderer, AdapterCtx } from '@object-ui/react'; + +const adapter = { find: async () => [] } as any; + +/** Registers a lazy `object-kanban` whose chunk lands only when you say so. */ +function lazyKanban() { + let land!: () => void; + const gate = new Promise((resolve) => { + land = resolve; + }); + ComponentRegistry.registerLazy( + 'object-kanban', + async () => { + await gate; + ComponentRegistry.register('object-kanban', () =>
, { + namespace: 'plugin-kanban', + }); + }, + { namespace: 'plugin-kanban', category: 'view' }, + ); + return land; +} + +function renderHtmlPage(source: string) { + return render( + + + , + ); +} + +beforeAll(async () => { + // Registers PageRenderer for `type:'home'`, which dispatches kind:'html'. + await import('../renderers'); +}, 30000); + +afterEach(() => { + ComponentRegistry.unregister('object-kanban', 'plugin-kanban'); + ComponentRegistry.unregister('object-kanban'); +}); + +describe('kind:\'html\' page with a lazily-registered block', () => { + it('compiles instead of failing the whole page, and renders the block once loaded', async () => { + const land = lazyKanban(); + + const { container, findByTestId } = renderHtmlPage( + ``, + ); + + // Before: 'HTML page failed to compile (2)' — ` is not an + // allowed component` + `is not a known component` — and no either. + expect(container.textContent).not.toContain('failed to compile'); + + land(); + expect(await findByTestId('kanban-double')).toBeTruthy(); + }); + + it('recovers a page that compiled before the block was registered', async () => { + // Nothing knows `object-kanban` yet, so this page genuinely cannot compile. + const { container } = renderHtmlPage(``); + expect(container.textContent).toContain('failed to compile'); + + // The plugin registers late — a legitimate case (a plugin installed at + // runtime, or any registration that races first paint). + ComponentRegistry.register('object-kanban', () =>
, { + namespace: 'plugin-kanban', + }); + + // Before: the memoised error panel outlived the registration, permanently. + await waitFor(() => expect(container.querySelector('[data-testid=kanban-double]')).toBeTruthy()); + expect(container.textContent).not.toContain('failed to compile'); + }); + + it('still rejects a tag nothing has registered at all', async () => { + const { container } = renderHtmlPage(``); + + // The negative control. Admitting lazy stubs must not turn the whitelist + // into "anything goes" — that whitelist is what keeps a parsed page from + // rendering arbitrary tags. + await waitFor(() => expect(container.textContent).toContain('failed to compile')); + expect(container.textContent).toContain('totally-not-a-block'); + }); +}); diff --git a/packages/components/src/renderers/layout/page.tsx b/packages/components/src/renderers/layout/page.tsx index f7aee8b9e..31738cfa6 100644 --- a/packages/components/src/renderers/layout/page.tsx +++ b/packages/components/src/renderers/layout/page.tsx @@ -366,16 +366,26 @@ function getJsxManifest() { // config's `.type` is always the namespaced form (Registry stores the bare // alias pointing at the namespaced type), so getAllConfigs() alone would // never admit the bare `` tag authors write. - const types = ComponentRegistry.getAllTypes(); - if (_jsxManifest === null || _jsxManifestSig !== types.length) { - const configs = types.map((t) => { - const cfg = ComponentRegistry.getConfig(t) as - | { namespace?: string; isContainer?: boolean; inputs?: unknown } - | undefined; - return { type: t, namespace: cfg?.namespace, isContainer: cfg?.isContainer, inputs: cfg?.inputs }; + // + // KNOWN types, not loaded ones. A lazily-registered block is a real member of + // this app's vocabulary; leaving it out of the whitelist rejected + // `` as "not an allowed component" and — because a compile + // error fails the WHOLE page, not just that node — took the entire page down + // with it, load-order dependently (objectui#2953, html tier). + // + // A stub has no `inputs` yet, so its props come back as `unknown-prop` + // WARNINGS rather than errors: the page compiles and renders, and the inner + // SchemaRenderer triggers the loader and swaps in the real block. Authoring + // time still gets full prop validation — `sdui.manifest.json` is generated + // with every plugin eagerly loaded, and asserts as much. + const version = ComponentRegistry.getVersion(); + if (_jsxManifest === null || _jsxManifestSig !== version) { + const configs = ComponentRegistry.getKnownTypes().map((t) => { + const meta = ComponentRegistry.getMeta(t); + return { type: t, namespace: meta?.namespace, isContainer: meta?.isContainer, inputs: meta?.inputs }; }); _jsxManifest = manifestFromConfigs(configs as unknown as Parameters[0]); - _jsxManifestSig = types.length; + _jsxManifestSig = version; } return _jsxManifest; } @@ -389,6 +399,13 @@ export const PageRenderer: React.FC<{ [key: string]: any; }> = ({ schema, className, ...props }) => { const pageType = schema.pageType || 'record'; + // A `kind:'html'` page compiles its source against the registry, and a + // compile error fails the whole page. Without this the failure is permanent + // for the session: `layoutElement` is memoised, so a page that compiled + // before the block it needs was registered kept rendering the cached error + // panel even after the plugin landed. Mirrors SchemaRenderer's subscription. + const [registryTick, bumpRegistryTick] = React.useReducer((n: number) => n + 1, 0); + React.useEffect(() => ComponentRegistry.subscribe(bumpRegistryTick), []); // Spec PageSchema declares `label` (required); `title` is the objectui // spelling. Dual-read so a spec-authored page still renders its header // (framework#1878 §3 naming-drift recheck). @@ -480,7 +497,12 @@ export const PageRenderer: React.FC<{ default: return ; } - }, [schema, pageType]); + // `registryTick` only matters to the kind:'html' branch above, which + // recompiles against the (now larger) whitelist. Every other branch just + // rebuilds the same element type with the same props, which React + // reconciles in place — no remount, no state loss (pinned by + // react-page-state.test.tsx). + }, [schema, pageType, registryTick]); // Full-bleed: a page whose `main` region is declared `width: 'full'` fills // the viewport with NO centered max-width cap — for dashboards / 大屏 / diff --git a/packages/core/src/registry/Registry.ts b/packages/core/src/registry/Registry.ts index a162b3c3d..3f4db2ab6 100644 --- a/packages/core/src/registry/Registry.ts +++ b/packages/core/src/registry/Registry.ts @@ -108,6 +108,14 @@ export class Registry { * load completes. */ private listeners = new Set<() => void>(); + /** + * Bumped whenever the set of KNOWN types changes — a registration, an + * unregistration, or a new lazy stub. A cheap cache key for consumers that + * derive something from the whole registry (whitelists, manifests, palettes) + * and need to rebuild when it grows. Counting types is not a substitute: a + * registration paired with an unregistration leaves the count untouched. + */ + private version = 0; /** * Register a component with optional namespace support. @@ -230,6 +238,10 @@ export class Registry { if (meta?.namespace && !meta?.skipFallback) { this.lazyEntries.set(type, entry); } + // Bump the version but do NOT notify: the set of KNOWN types grew, so + // whitelists and manifests must rebuild, but nothing new can render yet — + // waking every subscriber for each stub at boot would be pure churn. + this.version++; } /** @@ -274,6 +286,7 @@ export class Registry { } private notify() { + this.version++; for (const listener of this.listeners) { try { listener(); @@ -283,6 +296,14 @@ export class Registry { } } + /** + * Monotonic counter of changes to the set of known types. Rebuild anything + * derived from the whole registry when this moves — see {@link getKnownTypes}. + */ + getVersion(): number { + return this.version; + } + /** * Get a component by type. Supports both namespaced and non-namespaced lookups. * @@ -356,14 +377,50 @@ export class Registry { } /** - * Get all registered component types. - * + * Get all LOADED component types — what can be rendered right now. + * + * Pending `registerLazy` stubs are not included; use {@link getKnownTypes} + * for the question "is this a type this app knows about", which is what a + * whitelist or manifest wants (objectui#2953). + * * @returns Array of all component type identifiers */ getAllTypes(): string[] { return Array.from(this.components.keys()); } + /** + * Every type the registry can resolve — loaded registrations PLUS pending + * lazy stubs, deduped. Both the bare and namespaced keys are included, as + * they are in {@link getAllTypes}. + * + * This is the set to build a whitelist or a manifest from. Keying one off + * `getAllTypes()` instead makes it depend on which plugin chunks happen to + * have loaded: a lazily-registered block gets rejected as unknown, and + * whether it does varies by session (objectui#2953). + */ + getKnownTypes(): string[] { + return Array.from(new Set([...this.components.keys(), ...this.lazyEntries.keys()])); + } + + /** + * Metadata for `type` — from the loaded registration when there is one, + * otherwise from a pending `registerLazy` stub. + * + * Use this when you need to know what a type IS (namespace, container-ness, + * declared inputs). {@link getConfig} answers a different question — "can I + * render this right now" — and is deliberately loaded-only, since callers + * read `.component` off it. + * + * A stub carries only what `registerLazy` was given, so `inputs` is usually + * absent until the chunk loads. Consumers should treat that as "not yet + * known", not as "declares no props". + */ + getMeta(type: string, namespace?: string): ComponentMeta | undefined { + const key = namespace ? `${namespace}:${type}` : type; + return this.components.get(key) ?? this.lazyEntries.get(key)?.meta; + } + /** * Get all registered component configurations. * diff --git a/packages/core/src/registry/__tests__/Registry.test.ts b/packages/core/src/registry/__tests__/Registry.test.ts index ca6ccb598..10df100be 100644 --- a/packages/core/src/registry/__tests__/Registry.test.ts +++ b/packages/core/src/registry/__tests__/Registry.test.ts @@ -173,6 +173,83 @@ describe('Registry', () => { }); }); + describe('getKnownTypes() / getMeta() / getVersion() — lazy-aware reads (objectui#2953)', () => { + const loader = () => Promise.resolve(); + + it('getKnownTypes() includes pending lazy stubs; getAllTypes() does not', () => { + registry.register('button', () => 'b1'); + registry.registerLazy('object-kanban', loader, { namespace: 'plugin-kanban' }); + + // getAllTypes() answers "what can render right now" — the stub cannot. + expect(registry.getAllTypes()).not.toContain('object-kanban'); + // getKnownTypes() answers "what does this app know about" — the question + // a whitelist or a manifest is actually asking. Building one off + // getAllTypes() rejected lazily-registered blocks as unknown. + expect(registry.getKnownTypes()).toContain('object-kanban'); + expect(registry.getKnownTypes()).toContain('plugin-kanban:object-kanban'); + expect(registry.getKnownTypes()).toContain('button'); + }); + + it('getKnownTypes() dedupes a type that is both loaded and stubbed', () => { + registry.registerLazy('object-map', loader, { namespace: 'plugin-map' }); + registry.register('object-map', () => 'm', { namespace: 'plugin-map' }); + + const known = registry.getKnownTypes(); + expect(known.filter((t) => t === 'object-map')).toHaveLength(1); + }); + + it('getMeta() reads through to a stub, getConfig() stays loaded-only', () => { + registry.registerLazy('object-gantt', loader, { + namespace: 'plugin-gantt', + category: 'view', + isContainer: false, + }); + + expect(registry.getConfig('object-gantt')).toBeUndefined(); + expect(registry.getMeta('object-gantt')).toMatchObject({ + namespace: 'plugin-gantt', + category: 'view', + isContainer: false, + }); + // A stub has no declared inputs until its chunk lands — "not yet known", + // which consumers must not read as "declares no props". + expect(registry.getMeta('object-gantt')?.inputs).toBeUndefined(); + }); + + it('getMeta() prefers the loaded registration once the chunk lands', () => { + registry.registerLazy('object-chart', loader, { namespace: 'plugin-charts' }); + registry.register('object-chart', () => 'c', { + namespace: 'plugin-charts', + inputs: [{ name: 'type', type: 'string' }], + }); + + expect(registry.getMeta('object-chart')?.inputs).toHaveLength(1); + }); + + it('getMeta() honours an explicit namespace', () => { + registry.registerLazy('kanban', loader, { namespace: 'view' }); + + expect(registry.getMeta('kanban', 'view')).toBeTruthy(); + expect(registry.getMeta('kanban', 'nope')).toBeUndefined(); + }); + + it('getVersion() moves on every change to the known set', () => { + const start = registry.getVersion(); + registry.register('button', () => 'b1'); + const afterRegister = registry.getVersion(); + registry.registerLazy('object-map', loader, { namespace: 'plugin-map' }); + const afterLazy = registry.getVersion(); + registry.unregister('button'); + const afterUnregister = registry.getVersion(); + + // Counting types cannot substitute for this: a registration paired with + // an unregistration leaves the count untouched while the set changed. + expect(afterRegister).toBeGreaterThan(start); + expect(afterLazy).toBeGreaterThan(afterRegister); + expect(afterUnregister).toBeGreaterThan(afterLazy); + }); + }); + describe('getAllTypes() and getAllConfigs()', () => { it('should return all registered types including namespaced ones', () => { registry.register('button', () => 'b1');