fix: wrap page tree in BannerStateProvider (unbreaks @myst-theme 1.x rendering)#61
Merged
Conversation
@myst-theme 1.x moved banner visibility/height into a React context (`BannerStateProvider` in @myst-theme/providers); a child component calls `useBannerState()` during SSR. Our custom app/components/Page.tsx provider stack didn't include the provider, so every page threw "useBannerState must be used from within BannerStateProvider" and returned HTTP 500. `compile` + `prod:build` pass (it's a runtime/SSR error), and it had never surfaced because the deployed bundle is still v1.1.1 (@myst-theme 0.14) — the 1.x upgrade had never actually rendered. Wrap the tree in BannerStateProvider, mirroring upstream's page route. Found + validated with the new visual-regression harness (#60): all pages go 500 -> 200, notebook stream/error outputs render, and the diff vs the v1.1.1 baselines is a modest 2-4% (expected styling from the 0.14->1.3.0 upgrade). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses an SSR runtime crash introduced by upgrading to @myst-theme 1.x, where some layout code calls useBannerState() and now requires a BannerStateProvider to be present in the React provider stack.
Changes:
- Import
BannerStateProviderfrom@myst-theme/providers. - Wrap the main page render tree in
BannerStateProviderto preventuseBannerState must be used from within BannerStateProviderSSR failures.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…pper Addresses Copilot review: the provider was only on the happy path (Page), but ErrorBoundary -> ErrorPage also renders NavigationAndArticleWrapper, whose useSidebarHeight -> useBannerState would still 500 the error UI. Place BannerStateProvider inside NavigationAndArticleWrapper (used by both Page and ErrorPage) and drop the now-redundant wrap in Page.tsx, so every tree that renders the wrapper has the provider. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
mmcky
added a commit
that referenced
this pull request
Jun 4, 2026
Bump 1.1.1 -> 2.0.0 across package.json, package-lock.json and template.yml, and finalize the CHANGELOG [2.0.0] section. 2.0.0 is the @myst-theme 0.14 -> 1.x upgrade (breaking: new notebook output-node AST; Node engine >=20) plus the technical-review pass, the visual-regression harness (#60), and three pre-release fixes for regressions the 1.x upgrade introduced and caught before deploy: the BannerStateProvider 500 (#61), the full-bleed content column (#62), and the Remix 1.19 hydration reload loop (#63). CHANGELOG corrections vs the old [Unreleased] draft: drop the reverted #29 Remix 1.17->1.19 bump (2.0.0 stays on the v1.0.1 ~1.17.0 pin, so no net Remix change), correct the Node floor to >=20, and restate the Security note now that the older Remix v1 toolchain remains. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
2.0.0 deploy blocker.
@myst-theme1.x moved banner visibility/height into a React context (BannerStateProvider); a child component callsuseBannerState()during SSR. Our custom app/components/Page.tsx provider stack didn't include it, so every page threwuseBannerState must be used from within BannerStateProvider→ HTTP 500.Why it slipped through:
npm run compileandnpm run prod:buildboth pass.@myst-theme0.14), so the whole 0.14→1.x upgrade (Upgrade @myst-theme/* from 0.14.x to 1.1.2 #30, upgrade: @myst-theme 1.1.2 → 1.3.0 (latest upstream) + Node 24 toolchain #59) sat inmaingreen but unexecuted.Fix
Wrap the page tree in
BannerStateProvider(from@myst-theme/providers), mirroring upstream's page route.Validation (via the visual harness, #60)
/,/features,/notebookThis unblocks the 2.0.0 cut. Recommend landing #60 (harness) and this together; after 2.0.0 is accepted, re-baseline the snapshots to 2.0.0.
🤖 Generated with Claude Code