Uh oh!
There was an error while loading. Please reload this page.
fix(sdui): a react page keeps its state; a source that exports nothing fails loudly - #2984
Merged
Merged
Conversation
…g fails loudly
Writing the regression guard for objectui#2954's "latent hazard" — the note that
an unstable scope identity would turn into user-visible state loss — found it
was already real, on the default path.
`evaluatedSchema` was memoised on values rebuilt every render. SchemaRenderer
fell back to a fresh `{}` when no SchemaRendererProvider sat above it, and
`usePageVariables()` returned a brand-new object literal outside a
PageVariablesProvider. Both feed that memo's dependency list, so for any tree
without those providers it never hit: the schema was re-cloned and the
ExpressionEvaluator re-run on every render, and children got a new schema
identity every time. A kind:'react' page memoises its compiled source on that
identity, so the page was recompiled — a new page function, a new element type —
and React remounted it, silently discarding the user's useState. Every lazy
plugin's first load notified the registry and triggered it. Both fallbacks are
now module constants; the page-variables one is frozen through, since a shared
instance turns a stray write into a cross-consumer leak.
`react-page-state.test.tsx` pins the invariant at the symptom, not the memo:
state survives a parent re-render and survives a lazy block finishing its load
(the case a plausible-looking registry subscription in react-page.tsx would
break), and a genuinely changed source still recompiles.
`generateElement` now throws instead of rendering blank. The implicit
`export default` is only inserted when the source STARTS with JSX, a `function`
declaration, `()` or `class` — so the very common `const Page = () => …`
exported nothing and produced a blank page with no error reported anywhere. It
now throws with a message naming the fix, which ReactRunner's panel surfaces.
`export default null` still means "render nothing"; a non-component default
export throws too.
`PageSchema['kind']` matches @objectstack/spec. It declared 'full' | 'slotted'
while the renderer had shipped 'react' and 'html'/'jsx' since ADR-0080, reading
the field through a cast. The union now spells all five and the cast is gone.
Docs: new content/docs/guide/react-pages.md and a @object-ui/react-runtime
README. The package had neither, while being the tier AI-authored pages target —
the injected block scope, `Block`, `useAdapter`, the capability gate and the
accepted source shapes existed only in source comments.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N4mrr1ihhwnfEHFSWmGoMpThe latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
os-zhuang
marked this pull request as ready for review
July 30, 2026 07:09
Uh oh!
There was an error while loading. Please reload this page.
os-zhuang added a commit
that referenced
this pull request
Jul 30, 2026
…ts provider context (#3000) Audit of the remaining half of ReactKindPage's scope memo, [schema, adapter]. The schema half was the live bug fixed in #2984; this is the adapter half. The hosts are fine — both AdapterCtx.Provider call sites pass a stable value (AdapterProvider from useState, the console preview from a module constant), so there is no state loss in the shipped app. One real instance remained, one layer down: `dataSource={adapter ?? {}}` minted a fresh object every render while the adapter was still null (the window before the host connects). That is a context value and SchemaRendererProvider memoises on its identity, so every block inside the page had its schema re-cloned and its expressions re-run on each render. Now a module constant. The `adapter` dependency itself must stay, and is now pinned. It looks like the obvious thing to optimise away, but ReactRunner hands React the same element object while (code, scope) hold, and React bails out on an identical element reference — so recompiling is the ONLY path by which a new adapter reaches the blocks inside the page. Verified by removing it: every block stays pinned to the first adapter forever, with no error, just a dead data source. react-page-adapter.test.tsx pins both directions. Docs: the react-pages guide now states the host-side requirement — an adapter constructed inline on every render resets every react page on every render.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #2976 / #2979. Started as "write the regression guard for the state-loss hazard #2954 flagged" — the guard failed on first run, because the hazard was already real on the default path.
1.
evaluatedSchemawas memoised on values rebuilt every renderTwo fallbacks minted a fresh object per call:
Both feed the
evaluatedSchemauseMemodependency list. So for any tree without aSchemaRendererProvider/PageVariablesProviderabove it, that memo never hit: the schema was re-cloned and theExpressionEvaluatorre-run on every render, and every child got a new schema identity each time.For a
kind:'react'page that identity is the compile key. New identity → recompile → a new page function → a new element type → React remounts the subtree and the user'suseStateis gone. And sinceSchemaRenderersubscribes to the registry, every lazy plugin's first load fired a notify that triggered it.This is exactly the hazard #2954 described as "not yet observed":
Both fallbacks are now module constants. The page-variables one is frozen through — sharing one instance means a stray write would leak to every consumer outside a provider instead of being scoped to one render. There is no writer today (the setters are the API, and there they are no-ops); the freeze keeps it that way.
Worth noting the win is not limited to react pages: every
SchemaRendererwithout those providers was re-cloning its schema and re-running expression evaluation on every single render.2. The guard itself
packages/components/src/__tests__/react-page-state.test.tsxpins the invariant at the symptom, not the memo internals — the memo is an implementation detail, "the user's input survived" is the contract:ComponentRegistry.subscribe()inreact-page.tsxwould break, which is precisely what the comment there warns against and nothing enforced;sourcestill recompiles (stability must not mean staleness).3. A source that exports nothing now throws instead of rendering blank
normalizeCodeinserts the implicitexport defaultonly when the source starts with JSX, afunctiondeclaration,()orclass. So the form authors reach for most:…evaluated fine, exported nothing, and
generateElementreturnednull— a blank page, no error in the console, nothing in the page error panel. It now throws with a message naming the fix, whichReactRunner's panel surfaces (reachable since #2976).export default nullstill means "render nothing"; a default export that is not a component throws too.This was the open question I flagged before writing docs — documenting the tier required deciding whether that behaviour is contract or bug. It's a bug.
4.
PageSchema['kind']matches@objectstack/specThe spec has declared
full | slotted | html | jsx | reactsince ADR-0080.@object-ui/typesstill said'full' | 'slotted', andpage.tsxread the field through(schema as { kind?: string }).kindto dispatch on values the type denied existed. Per AGENTS #0 the type follows the spec: the union now spells all five and the cast is gone.5. Docs
@object-ui/react-runtimehad no README andcontent/docshad no page on either source-authored kind — the injected block scope,Block,useAdapter, the capability gate and the accepted source shapes existed only in source comments. That is the tier AI-authored pages are written against.content/docs/guide/react-pages.md— choosing between the executed and parsed tiers, the security gate, what's in scope (and why layout containers deliberately are not), flat props and thetype/specTypecollision,Block,useAdapter, source shapes, error handling. Added to the guide nav.packages/react-runtime/README.md— the API, the no-sandbox warning, and the stable-scope-identity requirement (an inlinescope={{...}}literal remounts the tree every render — the same trap as Implement visual designer for Object UI schemas #1, one layer up).Verification
697 files passed | 1 skipped,8201 tests passed | 24 skipped.turbo run type-checkacross the repo (76/76) — including after removing thekindcast.linton the touched packages: 0 errors.check-doc-links: the new page's links resolve. (One pre-existing break remains incontent/docs/core/enhanced-actions.mdx, untouched here and not gating — that workflow isworkflow_dispatch-only.)changeset:check: clean. Changeset included.🤖 Generated with Claude Code
https://claude.ai/code/session_01N4mrr1ihhwnfEHFSWmGoMp
Generated by Claude Code