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
34 changes: 34 additions & 0 deletions .changeset/react-page-lazy-scope-and-runner-memo.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
---
"@object-ui/core": patch
"@object-ui/react-runtime": patch
"@object-ui/components": patch
---

fix(sdui): lazily-registered public blocks reach a `kind:'react'` page's scope, and ReactRunner keeps the errors it catches

Two defects in the trusted `kind:'react'` page tier.

**objectui#2953 — the contract skipped lazy blocks.** `getPublicConfigs()`
resolved every curated `PUBLIC_BLOCKS` tag through `getConfig()`, which reads
loaded registrations only, so a block registered with `registerLazy()` was
absent from the contract until its plugin chunk happened to be imported. In
`apps/console` that silently dropped `object-kanban`, `object-calendar`,
`object-gantt`, `object-timeline`, `object-map` and `markdown` from every react
page's scope — writing `<ObjectKanban/>` threw `ReferenceError` even though the
tag is a first-class contract member, and whether it threw depended on load
order. `getPublicConfigs()` now resolves pending lazy stubs too, returning them
with `lazy: true` and no `component` (new `PublicComponentConfig` type); the
injected wrapper renders through `SchemaRenderer`, which triggers the loader and
shows its placeholder. `getConfig()` stays loaded-only by design.

**objectui#2954 — ReactRunner discarded its own error state.**
`getDerivedStateFromProps` re-transpiled and re-evaluated the page source on
every render and unconditionally set `error: null`. React runs it before the
re-render that follows `getDerivedStateFromError`, so the boundary threw away
the error it had just caught, rebuilt an identical throwing element, and the
throw escaped past its own `fallback` to the renderer's generic panel; `onError`
was gated on state that had already been cleared and never fired for a
compile-time error at all; and each compile minted a fresh page function — a new
element type — that remounted the subtree and wiped the page's `useState`. The
transpile+eval is now memoised on `(code, scope)`, errors persist until the
inputs actually change, and `onError` reports each error exactly once.
93 changes: 75 additions & 18 deletions packages/components/src/__tests__/react-page-scope.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,15 @@
* PUBLIC_BLOCKS or flip it to `isContainer` and it silently vanishes from every
* react page's scope while type-check, lint and build all stay green.
*
* These tests pin the three halves of that contract:
* These tests pin the four halves of that contract:
* 1. `list-view` / `object-form` stay eligible for injection;
* 2. an author writes `<ListView …/>` with FLAT props and no import, and the
* wrapper folds them into the block's `schema`;
* 3. an identifier that is genuinely absent fails LOUDLY — the negative
* control that keeps (2) from passing vacuously.
* 3. a block that is only *lazily* registered is in the scope too — the
* contract may not depend on which plugin chunks happen to be loaded
* (objectui#2953);
* 4. an identifier that is genuinely absent fails LOUDLY — the negative
* control that keeps (2) and (3) from passing vacuously.
*
* Registered stand-ins are used instead of the real plugin-list / plugin-form:
* `packages/components` sits BELOW the plugins in the dependency graph, and the
Expand All @@ -35,7 +38,7 @@ import { ComponentRegistry } from '@object-ui/core';
import { SchemaRenderer, AdapterCtx } from '@object-ui/react';

/** Props the stand-in blocks last received, for flat-prop assertions. */
const captured: { listView?: any; objectForm?: any } = {};
const captured: { listView?: any; objectForm?: any; kanban?: any } = {};

const adapter = { find: async () => [], getObjectSchema: async () => ({ name: 'showcase_project', fields: {} }) } as any;

Expand Down Expand Up @@ -73,6 +76,7 @@ afterAll(() => {
beforeEach(() => {
captured.listView = undefined;
captured.objectForm = undefined;
captured.kanban = undefined;
});

// ---------------------------------------------------------------------------
Expand All @@ -85,7 +89,7 @@ describe('kind:\'react\' scope eligibility', () => {
(tag) => {
const cfg = ComponentRegistry.getPublicConfigs().find((c: any) => c.type === tag);
// Missing from PUBLIC_BLOCKS, or marked isContainer, => dropped from the
// scope of every kind:'react' page (react-page.tsx:56-58).
// scope of every kind:'react' page (react-page.tsx:64-66).
expect(cfg).toBeTruthy();
expect((cfg as any).isContainer).toBeFalsy();
},
Expand All @@ -112,7 +116,7 @@ function Page() {
expect(await findByTestId('list-view-double')).toBeTruthy();
expect(await findByTestId('object-form-double')).toBeTruthy();
// The failure mode this guards: `ReferenceError: ListView is not defined`
// surfaced through ReactRunner's fallback (react-page.tsx:170-175).
// surfaced through ReactRunner's fallback (react-page.tsx:186-191).
expect(queryByText('React page error')).toBeNull();
});

Expand All @@ -125,7 +129,7 @@ function Page() {
await findByTestId('list-view-double');

// Flat JSX props are folded into the schema bag, and the discriminator wins
// the `type` slot (react-page.tsx:71-76).
// the `type` slot (react-page.tsx:79-84).
expect(captured.listView.schema).toMatchObject({
type: 'list-view',
objectName: 'showcase_project',
Expand All @@ -136,7 +140,7 @@ function Page() {
// apps/console/src/sdui-workbench-preview.tsx is built on onRowClick.
expect(typeof captured.listView.onRowClick).toBe('function');
// The wrapper stamps the live adapter in, since list-view reads dataSource
// from props rather than from context (react-page.tsx:52-55).
// from props rather than from context (react-page.tsx:61-63).
expect(captured.listView.schema.dataSource).toBe(adapter);
});

Expand All @@ -152,27 +156,80 @@ function Page() {
});

// ---------------------------------------------------------------------------
// 3. Negative control — absence must be loud
// 3. Lazily-registered blocks are contract members too (objectui#2953)
// ---------------------------------------------------------------------------

describe('kind:\'react\' scope — lazily-registered blocks', () => {
it('injects a curated block that is only registered lazily', async () => {
// How apps/console registers the heavy view plugins: a stub at boot, the
// chunk imported on first use. Six PUBLIC_BLOCKS tags live this way
// (object-kanban / -calendar / -gantt / -timeline / -map / markdown), and
// all six used to be missing from every react page's scope, because
// getPublicConfigs() resolved each curated tag through getConfig(), which
// reads loaded registrations only.
ComponentRegistry.registerLazy(
'object-kanban',
async () => {
ComponentRegistry.register(
'object-kanban',
(props: any) => {
captured.kanban = props;
return <div data-testid="kanban-double" />;
},
{ namespace: 'plugin-kanban' },
);
},
{ namespace: 'plugin-kanban', category: 'view' },
);

try {
// The tag IS in the contract before its chunk has been imported …
const cfg = ComponentRegistry.getPublicConfigs().find((c: any) => c.type === 'object-kanban');
expect(cfg).toBeTruthy();
expect((cfg as any).lazy).toBe(true);

// … so `<ObjectKanban>` resolves in the page source instead of throwing
// `ReferenceError: ObjectKanban is not defined`. The injected wrapper
// defers to SchemaRenderer, which fires the loader, shows the "Loading…"
// placeholder, and re-renders once the plugin registers for real.
const source = `
function Page() {
return <ObjectKanban objectName="showcase_project" groupBy="status" />;
}`;
const { findByTestId, queryByText } = renderReactPage(source);

expect(await findByTestId('kanban-double')).toBeTruthy();
expect(queryByText('React page error')).toBeNull();
expect(captured.kanban.schema).toMatchObject({ type: 'object-kanban', groupBy: 'status' });
} finally {
ComponentRegistry.unregister('object-kanban', 'plugin-kanban');
ComponentRegistry.unregister('object-kanban');
}
});
});

// ---------------------------------------------------------------------------
// 4. Negative control — absence must be loud
// ---------------------------------------------------------------------------

describe('kind:\'react\' unknown identifier', () => {
it('surfaces the ReferenceError instead of failing silently', async () => {
it('surfaces the ReferenceError in the page-level error panel', async () => {
const source = `
function Page() {
return <TotallyNotARegisteredBlock />;
}`;
const { container } = renderReactPage(source);
const { container, findByText } = renderReactPage(source);

// Proves the assertions above are meaningful: when a block really is absent
// from scope, the author sees this.
//
// Asserted on the message rather than on a specific panel, because the
// catching boundary is NOT the obvious one. ReactRunner is its own error
// boundary, but `getDerivedStateFromProps` re-transpiles on every render
// and unconditionally resets `error: null`, so the recovery render
// re-throws and the error escapes PAST the "React page error" fallback to
// SchemaRenderer's boundary ("Component "home" failed to render"). Pinning
// the panel would pin that quirk; pinning the message pins the contract.
// The panel is ReactRunner's own `fallback` (react-page.tsx:186-191).
// Pinning it also pins objectui#2954: `getDerivedStateFromProps` used to
// re-transpile on every render and reset `error: null`, so the recovery
// render rebuilt the same throwing element and the error escaped PAST this
// fallback to SchemaRenderer's boundary ("Component "home" failed to
// render") — the styled, page-specific panel was unreachable.
expect(await findByText('React page error')).toBeTruthy();
await waitFor(() =>
expect(container.textContent).toContain('TotallyNotARegisteredBlock is not defined'),
);
Expand Down
16 changes: 16 additions & 0 deletions packages/components/src/renderers/layout/react-page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,14 @@ function toPascal(tag: string): string {
// data/leaf blocks (non-containers) as prop-driven wrappers; layout containers
// are intentionally left out — in react mode the author composes layout with
// real HTML + Tailwind, not our schema-children renderers.
//
// Lazily-registered blocks (`object-kanban`, `object-map`, `markdown`, … — see
// apps/console/src/main.tsx) are in here too: `getPublicConfigs()` resolves
// `registerLazy` stubs, so the scope is complete at build time regardless of
// which plugin chunks have been imported (objectui#2953). Each wrapper defers
// to SchemaRenderer, which triggers the loader and renders the placeholder, so
// the scope itself never has to change once built — which is exactly what lets
// it stay identity-stable and keeps the page from remounting (objectui#2954).
function buildComponentScope(dataSource: unknown): Record<string, React.ComponentType<any>> {
const scope: Record<string, React.ComponentType<any>> = {};
const seen = new Set<string>();
Expand Down Expand Up @@ -123,6 +131,14 @@ export const ReactKindPage: React.FC<{ schema: any }> = ({ schema }) => {
};
}, [capabilityEnabled]);

// Keep this identity STABLE. ReactRunner recompiles the page whenever the
// scope identity changes, and every compile mints a fresh `Page` function —
// i.e. a new element type — which remounts the subtree and wipes the page's
// own `useState` (objectui#2954). That's why this deliberately does not
// subscribe to ComponentRegistry changes the way SchemaRenderer does: a lazy
// plugin finishing its registration notifies the registry, and rebuilding the
// scope there would reset every interactive page on the screen. It doesn't
// need to — `buildComponentScope` already sees lazy blocks (objectui#2953).
const scope = React.useMemo(
() => ({
...buildComponentScope(adapter),
Expand Down
66 changes: 61 additions & 5 deletions packages/core/src/registry/Registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,23 @@ export type ComponentConfig<T = any> = ComponentMeta & {
component: ComponentRenderer<T>;
};

/**
* A CONTRACT-surface entry (ADR-0080), as returned by
* {@link Registry.getPublicConfigs}.
*
* Same shape as {@link ComponentConfig} except `component` is absent while the
* entry is still a pending `registerLazy` stub: the plugin module has not been
* imported yet, so there is no renderer to hand out. Consumers render such an
* entry through `SchemaRenderer`, which triggers the loader and shows a
* placeholder in the meantime (objectui#2953).
*/
export type PublicComponentConfig<T = any> = ComponentMeta & {
type: string;
component?: ComponentRenderer<T>;
/** True while this entry is a `registerLazy` stub whose loader has not run. */
lazy?: boolean;
};

/**
* Lazy loader function used by `Registry.registerLazy`. The loader is invoked
* the first time a missing component type is requested through `getAsync`/the
Expand Down Expand Up @@ -298,7 +315,14 @@ export class Registry<T = any> {

/**
* Get component configuration by type with namespace support.
*
*
* LOADED registrations only — a type that exists solely as a `registerLazy`
* stub returns `undefined` here, because callers read `.component` off the
* result and a stub has no renderer until its loader runs. The *contract*
* question ("is this tag part of the public surface?") must not depend on
* load order, so {@link getPublicConfigs} resolves lazy stubs separately
* (objectui#2953). Use {@link hasLazy} / {@link loadLazy} to reach a stub.
*
* @param type - Component type (e.g., 'button' or 'ui:button')
* @param namespace - Optional namespace for lookup priority
* @returns Component configuration or undefined
Expand Down Expand Up @@ -349,30 +373,62 @@ export class Registry<T = any> {
return Array.from(this.components.values());
}

/**
* Resolve `tag` for the CONTRACT surface: the loaded registration when the
* component is already in the registry, otherwise the metadata of a pending
* `registerLazy` stub.
*
* A block registered lazily is a first-class member of the contract — the
* loader is recorded at boot and the plugin chunk is imported on first use —
* so contract membership must not hinge on whether that import happened to
* have run yet (objectui#2953).
*/
private getContractConfig(tag: string): PublicComponentConfig<T> | undefined {
const loaded = this.components.get(tag);
if (loaded) return loaded;
const entry = this.lazyEntries.get(tag);
if (!entry) return undefined;
// Mirror registerLazy's key derivation so the canonical `type` matches the
// one `register()` will store once the loader runs — that keeps the dedupe
// below stable across the load. `tag` may already carry the namespace
// (lazyEntries holds both keys; curated tags like `record:details` are
// themselves colon-shaped), so don't prefix it twice.
const ns = entry.meta?.namespace;
const canonical = ns && !tag.startsWith(`${ns}:`) ? `${ns}:${tag}` : tag;
return { ...entry.meta, type: canonical, lazy: true };
}

/**
* Get the curated PUBLIC-tier component configs (ADR-0080) — those registered
* with `tier: 'public'`. This is the contract/AI-vocabulary surface, a subset
* of the full rendering capability returned by {@link getAllConfigs}.
*
* Includes blocks that are only lazily registered so far; those come back
* with `lazy: true` and no `component` (see {@link PublicComponentConfig}).
*/
getPublicConfigs(): ComponentConfig<T>[] {
getPublicConfigs(): PublicComponentConfig<T>[] {
// Dedupe by the config's canonical (namespaced) `type` — a component is
// registered under both a bare and a namespaced key pointing at the same
// canonical type, and we want one contract entry per component.
const seenCanonical = new Set<string>();
const out: ComponentConfig<T>[] = [];
const add = (tag: string, cfg: ComponentConfig<T> | undefined): void => {
const out: PublicComponentConfig<T>[] = [];
const add = (tag: string, cfg: PublicComponentConfig<T> | undefined): void => {
if (!cfg || seenCanonical.has(cfg.type)) return;
seenCanonical.add(cfg.type);
// The contract surface is keyed by the bare/curated tag authors write,
// not the namespaced canonical stored on the config.
out.push({ ...cfg, type: tag });
};
// Curated contract list first (stable, reviewable order) …
for (const tag of PUBLIC_BLOCKS) add(tag, this.getConfig(tag));
for (const tag of PUBLIC_BLOCKS) add(tag, this.getContractConfig(tag));
// … plus any bare registration that opted in explicitly via `tier: 'public'`.
for (const [key, cfg] of this.components.entries()) {
if (cfg.tier === 'public' && !key.includes(':')) add(key, cfg);
}
// … and the same opt-in for stubs whose loader has not run yet.
for (const [key, entry] of this.lazyEntries.entries()) {
if (entry.meta?.tier === 'public' && !key.includes(':')) add(key, this.getContractConfig(key));
}
return out;
}

Expand Down
Loading
Loading