Repository navigation
perf(docs): cut live-example CSS cost with shared, deferred stylesheets - #444
Conversation
|
Warning Review limit reached
Next review available in: 5 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (7)
📒 Files selected for processing (11)
WalkthroughThe documentation site now copies Ionicons locally, shares constructable stylesheets across shadow roots, synchronizes Bulma theme variables, and defers live-preview rendering until previews approach the viewport. Unsupported environments retain inline-style and immediate-rendering fallbacks. ChangesDocumentation shadow previews
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant CodeBlockEnhancer
participant IntersectionObserver
participant ShadowLivePreview
participant ShadowRoot
participant LivePreview
CodeBlockEnhancer->>IntersectionObserver: observe live-preview slot
IntersectionObserver-->>CodeBlockEnhancer: report near-viewport entry
CodeBlockEnhancer->>ShadowLivePreview: mount live preview
ShadowLivePreview->>ShadowRoot: adopt shared stylesheets or add fallback styles
ShadowLivePreview->>LivePreview: render themed content
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Preview DeploymentPreview URL: https://317f9426.bestax.pages.dev |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Deep review — 0 blocking · 1 advisory
| # | Severity | Area | Finding | Location |
|---|---|---|---|---|
| 1 | 🔵 Advisory | Robustness | nomodule ionicons loader points at /ionicons/ionicons.js, which does not exist in the v8 dist (only ionicons.esm.js ships) — a dead reference, harmless in practice |
docs/src/theme/CodeBlock/ShadowLivePreview.js:60 |
Overall: The change is sound and the design is careful where it counts. The two load-bearing invariants — (a) the shared base sheets are parsed once at module scope and never mutated, only the per-instance theme sheet is written to, and (b) the styleSheets array identity is memoized so react-shadow's attach effect (useLayoutEffect(…, [n, j, d])) never re-fires and re-attachShadow()s on a color-mode toggle — both hold up under inspection of react-shadow@20.6.0's actual bundle. Color-mode switching is handled declaratively via the shared :host(.dark/.light) rules plus the host className, so cross-preview contamination is impossible. SSR is a non-issue: react-shadow renders an empty host on the server (children only portal in after attachShadow), and every consumer (ShadowLivePreview via React.lazy+BrowserOnly, the homepage ShadowPreview users via BrowserOnly) is client-only anyway, so the getSharedStyleSheets()→null SSR branch and the constructable-stylesheet fallback never produce a hydration mismatch. The riskiest area to eyeball is the deferral + Suspense interaction, but it degrades correctly: .live-preview-slot:empty keeps reserving height while <Suspense fallback={null}> resolves the chunk, and useInView degrades to "always in view" without IntersectionObserver.
I verified the dropped bulma-prefixed.min.css styles nothing live — every classPrefix="bulma-" occurrence is in a static tsx/scss fence or prose, never a live fence. I also confirmed the ionicons copy source (dist/ionicons, including svg/ and the p-*.entry.js lazy chunks) exists and that require.resolve('ionicons') → dist/ionicons resolves, so the CopyPlugin and /ionicons/ runtime resolution are correct. I could not run pnpm build (sandbox blocks it), so the human should confirm CI's build + check:conformance + check:urls are green — that is the empirical gate this review couldn't execute.
Residual risk — ways the "reparse per preview" / heavy-chunk failure class could still occur:
- A preview re-adopting sheets on every render (re-parse creep). Refuted:
useShadowStyleSheets/useInstanceStyleSheetmemoize on stable deps andgetSharedStyleSheets()returns a module-level singleton; react-shadow only assignsadoptedStyleSheetsonce at attach and its effect depdnever changes identity. - The CSS chunk sneaking back into a route that has no live fence. Refuted: the shadow root is behind
React.lazy(() => import('./ShadowLivePreview'))and only that module importsshadowStyles; nothing above the fold pulls it since the slot only mounts on intersection. - Font
url()breakage from stripping@font-faceinside the shadow sheets. Refuted by the PR's zero-pixel diffs plus the mechanism: the families are registered document-wide by the normal top-of-file CSS imports (webpack-rewritten URLs), and shadow content matches against those; the stripped rules were shadow-inert.
🏄 Total glass-off, offshore-wind kind of PR, dude — 462 stylesheets down to 9, previews chillin' below the lineup till you paddle near 'em, and not a single pixel wiped out. One dinky dead
nomoduleURL bobbing in the shallows, but it's not gonna close anybody out. Ship it. 🤙
|
@claude fix the bad reference spoitted in pr request |
|
Claude finished @allxsmith's task in 3m 10s —— View job Fixing the dead reference flagged in review
What I didThe deep review flagged I confirmed this directly against the installed package ( No Also checked for other occurrences of the pattern in the diff's files — none. (Note: Verified: Pushed as |
Preview DeploymentPreview URL: https://5414d638.bestax.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
docs/src/components/ShadowPreview/shadowStyles.js (1)
205-229: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider freezing the shared array.
getSharedStyleSheets()returns one live array to every preview. The docstring calls the sheets immutable, but nothing enforces that. If a future consumer pushes into the returned array instead of spreading it, every shadow root on the page is affected. Freeze the array to make the contract explicit.♻️ Proposed change
if (!sharedStyleSheets) { - sharedStyleSheets = shadowStyleSheets.map(css => { - const sheet = new CSSStyleSheet(); - sheet.replaceSync(css); - return sheet; - }); + sharedStyleSheets = Object.freeze( + shadowStyleSheets.map(css => { + const sheet = new CSSStyleSheet(); + sheet.replaceSync(css); + return sheet; + }) + ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/src/components/ShadowPreview/shadowStyles.js` around lines 205 - 229, Update getSharedStyleSheets so the memoized sharedStyleSheets array is frozen before it is returned, preserving the existing lazy initialization and fallback behavior while enforcing the documented immutable-array contract for all consumers.docs/src/theme/CodeBlock/ShadowTheme.js (1)
42-71: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueStabilize the effect dependencies.
themePropsis a rest object, so it has a new identity on every render.bulmaVarsis usually an inline object literal in the examples. The effect therefore re-runs after every render and callsupdateShadowTheme, which callsreplaceSyncand reparses the theme sheet. There is no render loop, becausereplaceSyncsets no React state, but the work is redundant. Build the variable map during render and depend on its serialized form.♻️ Proposed change
function ShadowTheme({ children, bulmaVars, isRoot, ...themeProps }) { const updateShadowTheme = React.useContext(ShadowThemeContext); - React.useEffect(() => { - // Only handle global theming (isRoot={true}) in shadow DOM - if (!isRoot || !updateShadowTheme) { - return; - } - - // Prepare the theme variables - const themeVars = {}; - - // Handle bulmaVars object - if (bulmaVars) { - Object.entries(bulmaVars).forEach(([key, value]) => { - themeVars[key] = value; - }); - } - - // Handle individual theme props - Object.entries(themeProps).forEach(([key, value]) => { - if (value !== null && value !== undefined) { - // Convert camelCase to kebab-case and add bulma prefix - const cssVar = `--bulma-${key - .replace(/([A-Z])/g, '-$1') - .toLowerCase()}`; - themeVars[cssVar] = value; - } - }); - - // Update the shadow DOM theme - updateShadowTheme(themeVars); - }, [bulmaVars, isRoot, themeProps, updateShadowTheme]); + const themeVars = { ...(bulmaVars ?? {}) }; + Object.entries(themeProps).forEach(([key, value]) => { + if (value !== null && value !== undefined) { + // Convert camelCase to kebab-case and add the bulma prefix. + themeVars[`--bulma-${key.replace(/([A-Z])/g, '-$1').toLowerCase()}`] = + value; + } + }); + const themeVarsKey = JSON.stringify(themeVars); + + React.useEffect(() => { + // Only handle global theming (isRoot={true}) in the shadow root. + if (!isRoot || !updateShadowTheme) { + return; + } + updateShadowTheme(JSON.parse(themeVarsKey)); + }, [isRoot, themeVarsKey, updateShadowTheme]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/src/theme/CodeBlock/ShadowTheme.js` around lines 42 - 71, Stabilize the ShadowTheme effect by building the complete CSS variable map during render, including bulmaVars and filtered themeProps, then derive a serialized representation for dependency tracking. Update the effect around updateShadowTheme to depend on that serialized map (alongside isRoot and updateShadowTheme) and apply the corresponding variable map, avoiding direct dependencies on the rest-object themeProps and inline bulmaVars identities.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/src/theme/CodeBlock/index.js`:
- Around line 76-107: Update useInView to lazily initialize inView from whether
IntersectionObserver is unavailable, removing the synchronous setInView(true)
fallback from the effect. Preserve the existing observer-based transition and
cleanup behavior for supported browsers, relying on DeferredShadowPreview’s
BrowserOnly rendering context.
In `@docs/src/theme/CodeBlock/ShadowLivePreview.js`:
- Around line 35-53: Update the color-mode handling around updateShadowTheme and
its effect so color-mode updates no longer replace or discard variables written
by ShadowTheme for <Theme isRoot>. Keep the two variable sets separate, either
by using a dedicated instance sheet or by merging both variable maps before each
replaceSync, and revise the adjacent comment to accurately describe the
preserved behavior.
---
Nitpick comments:
In `@docs/src/components/ShadowPreview/shadowStyles.js`:
- Around line 205-229: Update getSharedStyleSheets so the memoized
sharedStyleSheets array is frozen before it is returned, preserving the existing
lazy initialization and fallback behavior while enforcing the documented
immutable-array contract for all consumers.
In `@docs/src/theme/CodeBlock/ShadowTheme.js`:
- Around line 42-71: Stabilize the ShadowTheme effect by building the complete
CSS variable map during render, including bulmaVars and filtered themeProps,
then derive a serialized representation for dependency tracking. Update the
effect around updateShadowTheme to depend on that serialized map (alongside
isRoot and updateShadowTheme) and apply the corresponding variable map, avoiding
direct dependencies on the rest-object themeProps and inline bulmaVars
identities.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 58c0f0bd-c891-4f08-a358-ad7e1fee4846
⛔ Files ignored due to path filters (7)
docs/static/css/ionicons.min.cssis excluded by!**/*.min.cssdocs/static/fonts/ionicons.eotis excluded by!**/*.eotdocs/static/fonts/ionicons.svgis excluded by!**/*.svgdocs/static/fonts/ionicons.ttfis excluded by!**/*.ttfdocs/static/fonts/ionicons.woffis excluded by!**/*.woffdocs/static/fonts/ionicons.woff2is excluded by!**/*.woff2pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (11)
docs/docusaurus.config.jsdocs/package.jsondocs/plugins/local-ionicons.jsdocs/src/components/ShadowPreview/index.jsdocs/src/components/ShadowPreview/shadowStyles.jsdocs/src/components/ShadowPreview/useShadowStyleSheets.jsdocs/src/css/custom.cssdocs/src/theme/CodeBlock/ShadowLivePreview.jsdocs/src/theme/CodeBlock/ShadowTheme.jsdocs/src/theme/CodeBlock/index.jsdocs/static/_headers
| function useInView(rootMargin = '600px') { | ||
| const ref = React.useRef(null); | ||
| const [inView, setInView] = React.useState(false); | ||
|
|
||
| // Update shadow DOM class and color variables when color mode changes | ||
| React.useEffect(() => { | ||
| if (shadowRef.current) { | ||
| // Update the host element class to reflect current color mode | ||
| shadowRef.current.className = `live-preview ${colorMode}`; | ||
|
|
||
| // Apply explicit color mode variables for better consistency | ||
| if (updateShadowTheme) { | ||
| const colorModeVars = | ||
| colorMode === 'dark' | ||
| ? { | ||
| '--bulma-text-strong': 'hsl(0, 0%, 96%)', | ||
| '--bulma-label-color': 'hsl(0, 0%, 96%)', | ||
| '--bulma-text': 'hsl(0, 0%, 96%)', | ||
| '--bulma-text-weak': 'hsl(0, 0%, 71%)', | ||
| '--bulma-background': 'hsl(0, 0%, 14%)', | ||
| '--bulma-surface': 'hsl(0, 0%, 21%)', | ||
| '--bulma-border': 'hsl(0, 0%, 29%)', | ||
| '--bulma-border-weak': 'hsl(0, 0%, 24%)', | ||
| } | ||
| : { | ||
| '--bulma-text-strong': 'hsl(221, 14%, 21%)', | ||
| '--bulma-label-color': 'hsl(221, 14%, 21%)', | ||
| '--bulma-text': 'hsl(221, 14%, 31%)', | ||
| '--bulma-text-weak': 'hsl(221, 14%, 41%)', | ||
| '--bulma-background': 'hsl(0, 0%, 100%)', | ||
| '--bulma-surface': 'hsl(0, 0%, 98%)', | ||
| '--bulma-border': 'hsl(221, 14%, 86%)', | ||
| '--bulma-border-weak': 'hsl(221, 14%, 93%)', | ||
| }; | ||
|
|
||
| updateShadowTheme(colorModeVars); | ||
| } | ||
| if (inView) { | ||
| return undefined; | ||
| } | ||
| if (typeof IntersectionObserver === 'undefined') { | ||
| setInView(true); | ||
| return undefined; | ||
| } | ||
| const node = ref.current; | ||
| if (!node) { | ||
| return undefined; | ||
| } | ||
| }, [colorMode, updateShadowTheme]); | ||
|
|
||
| React.useEffect(() => { | ||
| // Set up the shadow style element reference after component mounts | ||
| const setupShadowStyleElement = () => { | ||
| if (shadowRef.current && shadowRef.current.shadowRoot) { | ||
| const styleElement = | ||
| shadowRef.current.shadowRoot.querySelector('#theme-vars'); | ||
| if (styleElement) { | ||
| setThemeStyleElement(styleElement); | ||
| return styleElement; | ||
| const observer = new IntersectionObserver( | ||
| entries => { | ||
| if (entries.some(entry => entry.isIntersecting)) { | ||
| setInView(true); | ||
| observer.disconnect(); | ||
| } | ||
| } | ||
| return null; | ||
| }; | ||
|
|
||
| // Try immediately, and if not ready, try with a short delay | ||
| let styleElement = setupShadowStyleElement(); | ||
| let timeoutId = null; | ||
|
|
||
| if (!styleElement) { | ||
| const SHADOW_DOM_INIT_TIMEOUT_MS = 50; | ||
| timeoutId = setTimeout(() => { | ||
| styleElement = setupShadowStyleElement(); | ||
| }, SHADOW_DOM_INIT_TIMEOUT_MS); // Give shadow DOM time to initialize | ||
| } | ||
| }, | ||
| { rootMargin } | ||
| ); | ||
| observer.observe(node); | ||
| return () => observer.disconnect(); | ||
| }, [inView, rootMargin]); | ||
|
|
||
| // Cleanup on unmount | ||
| return () => { | ||
| if (timeoutId) clearTimeout(timeoutId); | ||
| setThemeStyleElement(null); | ||
| }; | ||
| }, []); | ||
| return [ref, inView]; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Resolve the ESLint react-hooks/set-state-in-effect error.
ESLint reports an error at Line 85 for the synchronous setInView(true) inside the effect. The coding guidelines require ESLint to pass before submission, so this blocks the change. Initialize the state lazily instead. DeferredShadowPreview renders only inside BrowserOnly, so reading IntersectionObserver during render is safe here.
🔧 Proposed fix
function useInView(rootMargin = '600px') {
const ref = React.useRef(null);
- const [inView, setInView] = React.useState(false);
+ // Degrade to "always in view" where IntersectionObserver is missing.
+ const [inView, setInView] = React.useState(
+ () => typeof IntersectionObserver === 'undefined'
+ );
React.useEffect(() => {
if (inView) {
return undefined;
}
- if (typeof IntersectionObserver === 'undefined') {
- setInView(true);
- return undefined;
- }
const node = ref.current;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function useInView(rootMargin = '600px') { | |
| const ref = React.useRef(null); | |
| const [inView, setInView] = React.useState(false); | |
| // Update shadow DOM class and color variables when color mode changes | |
| React.useEffect(() => { | |
| if (shadowRef.current) { | |
| // Update the host element class to reflect current color mode | |
| shadowRef.current.className = `live-preview ${colorMode}`; | |
| // Apply explicit color mode variables for better consistency | |
| if (updateShadowTheme) { | |
| const colorModeVars = | |
| colorMode === 'dark' | |
| ? { | |
| '--bulma-text-strong': 'hsl(0, 0%, 96%)', | |
| '--bulma-label-color': 'hsl(0, 0%, 96%)', | |
| '--bulma-text': 'hsl(0, 0%, 96%)', | |
| '--bulma-text-weak': 'hsl(0, 0%, 71%)', | |
| '--bulma-background': 'hsl(0, 0%, 14%)', | |
| '--bulma-surface': 'hsl(0, 0%, 21%)', | |
| '--bulma-border': 'hsl(0, 0%, 29%)', | |
| '--bulma-border-weak': 'hsl(0, 0%, 24%)', | |
| } | |
| : { | |
| '--bulma-text-strong': 'hsl(221, 14%, 21%)', | |
| '--bulma-label-color': 'hsl(221, 14%, 21%)', | |
| '--bulma-text': 'hsl(221, 14%, 31%)', | |
| '--bulma-text-weak': 'hsl(221, 14%, 41%)', | |
| '--bulma-background': 'hsl(0, 0%, 100%)', | |
| '--bulma-surface': 'hsl(0, 0%, 98%)', | |
| '--bulma-border': 'hsl(221, 14%, 86%)', | |
| '--bulma-border-weak': 'hsl(221, 14%, 93%)', | |
| }; | |
| updateShadowTheme(colorModeVars); | |
| } | |
| if (inView) { | |
| return undefined; | |
| } | |
| if (typeof IntersectionObserver === 'undefined') { | |
| setInView(true); | |
| return undefined; | |
| } | |
| const node = ref.current; | |
| if (!node) { | |
| return undefined; | |
| } | |
| }, [colorMode, updateShadowTheme]); | |
| React.useEffect(() => { | |
| // Set up the shadow style element reference after component mounts | |
| const setupShadowStyleElement = () => { | |
| if (shadowRef.current && shadowRef.current.shadowRoot) { | |
| const styleElement = | |
| shadowRef.current.shadowRoot.querySelector('#theme-vars'); | |
| if (styleElement) { | |
| setThemeStyleElement(styleElement); | |
| return styleElement; | |
| const observer = new IntersectionObserver( | |
| entries => { | |
| if (entries.some(entry => entry.isIntersecting)) { | |
| setInView(true); | |
| observer.disconnect(); | |
| } | |
| } | |
| return null; | |
| }; | |
| // Try immediately, and if not ready, try with a short delay | |
| let styleElement = setupShadowStyleElement(); | |
| let timeoutId = null; | |
| if (!styleElement) { | |
| const SHADOW_DOM_INIT_TIMEOUT_MS = 50; | |
| timeoutId = setTimeout(() => { | |
| styleElement = setupShadowStyleElement(); | |
| }, SHADOW_DOM_INIT_TIMEOUT_MS); // Give shadow DOM time to initialize | |
| } | |
| }, | |
| { rootMargin } | |
| ); | |
| observer.observe(node); | |
| return () => observer.disconnect(); | |
| }, [inView, rootMargin]); | |
| // Cleanup on unmount | |
| return () => { | |
| if (timeoutId) clearTimeout(timeoutId); | |
| setThemeStyleElement(null); | |
| }; | |
| }, []); | |
| return [ref, inView]; | |
| } | |
| function useInView(rootMargin = '600px') { | |
| const ref = React.useRef(null); | |
| // Degrade to "always in view" where IntersectionObserver is missing. | |
| const [inView, setInView] = React.useState( | |
| () => typeof IntersectionObserver === 'undefined' | |
| ); | |
| React.useEffect(() => { | |
| if (inView) { | |
| return undefined; | |
| } | |
| const node = ref.current; | |
| if (!node) { | |
| return undefined; | |
| } | |
| const observer = new IntersectionObserver( | |
| entries => { | |
| if (entries.some(entry => entry.isIntersecting)) { | |
| setInView(true); | |
| observer.disconnect(); | |
| } | |
| }, | |
| { rootMargin } | |
| ); | |
| observer.observe(node); | |
| return () => observer.disconnect(); | |
| }, [inView, rootMargin]); | |
| return [ref, inView]; | |
| } |
🧰 Tools
🪛 ESLint
[error] 85-85: Error: Calling setState synchronously within an effect can trigger cascading renders
Effects are intended to synchronize state between React and external systems such as manually updating the DOM, state management libraries, or other platform APIs. In general, the body of an effect should do one or both of the following:
- Update external systems with the latest state from React.
- Subscribe for updates from some external system, calling setState in a callback function when external state changes.
Calling setState synchronously within an effect body causes cascading renders that can hurt performance, and is not recommended. (https://react.dev/learn/you-might-not-need-an-effect).
83 | }
84 | if (typeof IntersectionObserver === 'undefined') {
85 | setInView(true);
| ^^^^^^^^^ Avoid calling setState() directly within an effect
86 | return undefined;
87 | }
88 | const node = ref.current;
(react-hooks/set-state-in-effect)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/src/theme/CodeBlock/index.js` around lines 76 - 107, Update useInView to
lazily initialize inView from whether IntersectionObserver is unavailable,
removing the synchronous setInView(true) fallback from the effect. Preserve the
existing observer-based transition and cleanup behavior for supported browsers,
relying on DeferredShadowPreview’s BrowserOnly rendering context.
Sources: Coding guidelines, Linters/SAST tools
| const updateShadowTheme = React.useCallback( | ||
| themeVars => { | ||
| const css = hostRule(themeVars); | ||
| if (themeSheet) { | ||
| themeSheet.replaceSync(css); | ||
| } else if (themeStyleRef.current) { | ||
| themeStyleRef.current.textContent = css; | ||
| } | ||
| }, | ||
| [themeSheet] | ||
| ); | ||
|
|
||
| // Re-apply the color-mode variables whenever the site theme flips. The | ||
| // declarative :host(.dark)/:host(.light) rules in colorModeStyles cover the | ||
| // common case; this keeps them pinned when an example's own <Theme isRoot> | ||
| // has already written to the instance sheet. | ||
| React.useEffect(() => { | ||
| updateShadowTheme(colorModeVars[colorMode] ?? colorModeVars.light); | ||
| }, [colorMode, updateShadowTheme]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
The color-mode effect replaces the whole instance sheet, including <Theme isRoot> variables.
updateShadowTheme calls themeSheet.replaceSync(css), which replaces the entire sheet content instead of merging declarations. Two writers share this one sheet: this color-mode effect and the effect in ShadowTheme. On a color-mode flip, this effect writes only the 8 colorModeVars entries and discards whatever an example's <Theme isRoot> had set. The comment states the opposite.
Today the loss is usually invisible, because ShadowTheme's effect re-runs on almost every render and rewrites its variables. That masking disappears if those dependencies are stabilized. Keep the two variable sets separate, for example by giving the color-mode variables their own instance sheet, or by merging both maps before each replaceSync. Also correct the comment on Lines 47-50.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/src/theme/CodeBlock/ShadowLivePreview.js` around lines 35 - 53, Update
the color-mode handling around updateShadowTheme and its effect so color-mode
updates no longer replace or discard variables written by ShadowTheme for <Theme
isRoot>. Keep the two variable sets separate, either by using a dedicated
instance sheet or by merging both variable maps before each replaceSync, and
revise the adjacent comment to accurately describe the preserved behavior.
Each `live` fence renders into its own shadow root, and each root was given
its own eleven inline <style> children carrying the whole Bulma + icon-font
set (~1.85 MB of CSS). Nothing was shared between roots, so the browser
reparsed the lot per preview: the heaviest page, api/form/datetime/timeinput,
built 462 CSSStyleSheet objects from ~78 MB of CSS text at load.
Parse the sheets once at module scope into constructable CSSStyleSheets and
hand the same objects to every shadow root via react-shadow's `styleSheets`
prop (which assigns shadowRoot.adoptedStyleSheets). Only the per-instance
theme-variable sheet is unique per preview. That page now parses 9 shared
sheets plus 42 tiny :host{} rules. Browsers without constructable stylesheets
(Safari < 16.4) and the SSR pass fall back to the previous <style> children,
which renders pixel-identically.
Three things follow from the rework:
- Drop bulma-prefixed.min.css. At 726 KB it was 39% of the payload, and the
only `classPrefix="bulma-"` left in the docs sits in a non-live tsx fence in
the 4-to-5 migration guide, so it styled nothing.
- Strip @font-face from every shadow sheet, not just material-icons. raw-loader
does not rewrite url(), so those rules keep package-relative paths. They are
inert rather than broken — browsers do not register @font-face declared
inside a shadow tree (verified in Chromium 141; the previews issue no failing
font requests before or after this change), and the families are registered
document-wide by the plain CSS imports instead. The strip runs at module
init, so the raw text still ships in the chunk; it drops ~14 inert rules from
the shared sheets once per page load, not once per preview. The point is
uniformity rather than volume: material-icons already did this, with the same
reasoning in its comment, and the other three never got the fix.
- Move the shadow root into its own module behind a dynamic import, so the CSS
no longer rides in a chunk every route loads. The editor still server-renders
— only the preview, which is browser-only anyway, is deferred.
Measured against the production build (Chromium, per page, scripts only):
no live fences 3210 KB -> 1076 KB (-66%)
42 live examples 3254 KB -> 2545 KB (-22%)
homepage 2914 KB -> 2204 KB (-24%)
Total build JS 10,097,034 -> 8,692,021 bytes.
Verified identical rendering: full-page screenshots of button, field,
usebulmaclasses, color-palette and the homepage, in both color modes, differ
by zero pixels before and after — and again with constructable stylesheets
disabled to exercise the fallback.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DJCcqoU4YxnQrrs2BQqG6g
Sharing the stylesheet set removed the CSS cost of having many previews on a page, but each one still mounted eagerly: api/form/datetime/timeinput ran 42 react-live evaluations and attached 42 shadow roots before first paint, when the first example does not appear until 6063px down the page. Wrap each preview in a slot that mounts it once an IntersectionObserver says it is within 600px of the viewport, then stops observing — previews mount once and stay mounted. Where IntersectionObserver is missing the slot reports "in view" immediately, so behaviour is unchanged. Because the shadow root is already behind a dynamic import, nothing above the fold pulls the stylesheet chunk at all. Initial scripts on that page: 2545 KB -> 1120 KB Examples that are actually on screen still mount on load — guides/library/ elements, whose first example sits at 788px, mounts two immediately. `.live-preview-slot:empty` reserves a little height so a page of not-yet- mounted examples keeps a sensible scroll length; the rule stops applying the moment a preview mounts. Verified no rendering change: after scrolling each page to mount everything, full-page screenshots of button, field, usebulmaclasses, color-palette and the homepage in both color modes differ from the pre-deferral build by zero pixels. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DJCcqoU4YxnQrrs2BQqG6g
…ssets Four related bits of housekeeping around the live-example assets. ionicons v8 was fetched from unpkg.com at runtime even though `ionicons` is a declared dependency — a third-party availability and privacy dependency on every page with a live preview, pinned only by a URL. It cannot simply be imported, because the Stencil bundle resolves its own lazy chunks and per-icon SVGs relative to the loader's URL, so plugins/local-ionicons.js copies dist/ionicons into the build and the script tags now point at /ionicons/. The per-icon SVGs are still fetched on demand; a preview pulls three files. static/css/ionicons.min.css and static/fonts/ionicons.* were Ionicons v4 — the icon-font era. The `stylesheets` entry that referenced them has been commented out in docusaurus.config.js for as long as the v8 web components have been in use, so ~700 KB shipped on every deploy referenced by nothing. Removed, along with the commented-out config block. There was no _headers file, so the site ran entirely on Cloudflare Pages' defaults. Docusaurus fingerprints everything under assets/, which can now be cached immutably, while HTML and the generated llms.txt / llms-full.txt must revalidate so agents and readers do not get a stale API surface after a deploy. `bulma` was imported by docs/src but declared only at the workspace root, so the docs build depended on hoisting — exactly what the isolated linker is configured to catch. Declared it, plus copy-webpack-plugin for the new plugin. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DJCcqoU4YxnQrrs2BQqG6g
v8's dist only ships the ESM loader (ionicons.esm.js) — there is no legacy ionicons.js bundle to fall back to, so the nomodule <script> tag pointed at a nonexistent asset. Confirmed against the installed ionicons@8.0.13 package: dist/ionicons/ contains only *.esm.js and *.entry.js chunks, no ionicons.js. Co-authored-by: Alex Smith <allxsmith@users.noreply.github.com>
4c2eeb7 to
82be3e4
Compare
Rebased onto main — conflicts resolvedWas 28 commits behind (branch point One real conflict, in
Resolved by keeping both.
The composition worth checking
Non-live code blocks fall through to
|
Preview DeploymentPreview URL: https://84aa16f4.bestax.pages.dev |
|
🎉 This PR is included in version 4.0.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 5.8.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 2.0.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
2 participants
Pull Request
Description
Every
livefence renders into its own shadow root, and each root was given its own eleven inline<style>children carrying the whole Bulma + icon-font set (~1.85 MB of CSS). Nothing was shared between roots, so the browser reparsed the lot per preview —api/form/datetime/timeinputbuilt 462CSSStyleSheetobjects from ~78 MB of CSS text at load. The same payload also rode in the chunk every route loads, including pages with no live fence at all.Three commits, each independently reviewable:
perf(docs)— share one parsed stylesheet set. Parse the sheets once at module scope into constructableCSSStyleSheets and hand the same objects to every shadow root viareact-shadow'sstyleSheetsprop (which assignsshadowRoot.adoptedStyleSheets). Only the per-instance theme-variable sheet is unique per preview. That page now parses 9 shared sheets plus 42 tiny:host{}rules. Also dropsbulma-prefixed.min.css(726 KB, 39% of the payload, styling nothing), and moves the shadow root behind a dynamic import so the CSS no longer rides every route.perf(docs)— defer previews until they scroll into view. AnIntersectionObserverslot mounts each preview once it is within 600px of the viewport. Since the shadow root is already behind a dynamic import, nothing above the fold pulls the CSS chunk at all.refactor(docs)— serve ionicons from this origin, prune dead assets. ionicons v8 was fetched fromunpkg.comat runtime despite being a declared dependency; it is now copied into the build by a small plugin. Deletes ~700 KB of Ionicons v4 CSS/fonts referenced only by a commented-out config block, adds a_headerscache policy, and declaresbulma+copy-webpack-pluginindocs/package.json(both previously resolved only via hoisting).Please provide a summary of the changes and which package(s) are affected.
@allxsmith/bestax-bulma)create-bestax)@allxsmith/bestax-docs)Related Issue(s)
Closes #443
Type of Change
Checklist
CLAUDE.mdfiles are updatedNotes on the unchecked boxes, since they are deliberate rather than skipped:
docshas no component test suite.pnpm check:conformance(all 10 sub-checks) passes.Screenshots / Demos
No screenshots, because the point is that there is nothing to see — rendering is byte-identical.
Full-page screenshots at 1280×2400 of
button,field,usebulmaclasses,color-paletteand the homepage, in both color modes, differ by zero pixels before and after. Verified three times over:<style>fallback,30 comparisons, 0 differing pixels.
Additional Context
Measured against the production build (Chromium 141, per page, scripts only):
timeinput.md)Total build JS 10,097,034 → 8,692,021 bytes. Stylesheets parsed on
timeinput.md: 462 → 9 shared.The shadow CSS stays inline-in-JS rather than becoming a
.cssfile, which is what keeps it cheap: it compiles to one content-hashed chunk (~198 KB gzip) shared by all ~110 live-example pages, fetched once per visit and then served from cache — andassets/*is now markedimmutableby the new_headers, so there is not even a revalidation round-trip.Two things I want to flag rather than bury:
@font-facestrip applied to the shadow sheets is runtime-only — it runs at module init, so the raw text still ships in the chunk. It drops ~14 inert rules from the shared sheets once per page load, not per preview. Its value is uniformity (materialIconsStylesalready did this; the other three sheets never got the same fix), not bytes. Those rules were inert, not broken: browsers do not register@font-facedeclared inside a shadow tree, verified in Chromium 141, and the previews issue no failing font requests before or after..live-preview-slot:emptyreserves 6rem so a page of deferred examples keeps a sensible scroll length, and previews genuinely on screen still mount on load (guides/library/elementsmounts two immediately, first slot at 788px).Out of scope, worth a separate issue:
assets/fonts/is 18 MB, of which Material Symbols alone is 12.8 MB. It is lazy — browsers fetch only the faces actually used — but subsetting those fonts is a far bigger lever on page weight than anything in the CSS pipeline. Untouched here.Gates:
pnpm allgreen (21/21) andpnpm check:conformancegreen (all 10 sub-checks — note it is a separate CI step, not part ofpnpm all).pnpm check:urlswas not run locally; it needs network access to bestax.io, which the sandbox blocks. It will run in CI.Generated by Claude Code
Summary by CodeRabbit