-
-
Notifications
You must be signed in to change notification settings - Fork 199
fix(theme): improve SSR compatibility #2934
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
✅ Deploy Preview for rspress-v2 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR improves SSR (Server-Side Rendering) compatibility by fixing issues in the useStorageValue hook and PageTabs component. The changes address SSR warnings from useLayoutEffect usage, add browser environment guards, and eliminate a hydration mismatch caused by a global render counter.
Key changes:
- Replace
useLayoutEffectwithuseEffectand add SSR guard inuseStorageValuehook - Remove global
renderCountForTocUpdatecounter and conditional TOC update logic fromPageTabscomponent
Reviewed changes
Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/core/src/theme/logic/useStorageValue.ts | Migrates from useLayoutEffect to useEffect with SSR guard for localStorage access, updates dependency array to include key |
| packages/core/src/theme/components/PageTabs/index.tsx | Removes global counter and conditional hidden <h2> rendering that caused hydration mismatches |
Comments suppressed due to low confidence (1)
packages/core/src/theme/logic/useStorageValue.ts:45
- The
setValuecallback accesseslocalStorageanddispatchEventwithout checking ifwindowis defined. This will cause errors during SSR when this function is called. Add a guard to checktypeof window !== 'undefined'before accessing browser APIs.
const setValue = useCallback(
(value: T) => {
const next = value;
if (next == null) {
localStorage.removeItem(key);
} else {
localStorage.setItem(key, next.toString());
}
setValueInternal(next);
dispatchEvent(
// send custom event to communicate within same page
// importantly this should not be a StorageEvent since those cannot
// be constructed with a non-built-in storage area
new CustomEvent<{ key: string; newValue: T }>(SYNC_STORAGE_EVENT_NAME, {
detail: {
key,
newValue: next,
},
}),
);
},
[key],
);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
b23de65 to
0104c1d
Compare
Rsdoctor Bundle Diff AnalysisFound 3 projects in monorepo, 1 project with changes. 📊 Quick Summary
📋 Detailed Reports (Click to expand)📁 webPath:
📦 Download Diff Report: web Bundle Diff Generated by Rsdoctor GitHub Action |
Summary
useLayoutEffectwithuseEffectinuseStorageValueto avoid SSR warningstypeof windowcheck) inuseStorageValuerenderCountForTocUpdatecounter inPageTabsthat causes hydration mismatchBackground
useStorageValue issues
The
useStorageValuehook had two SSR compatibility issues:useLayoutEffectwhich causes warnings during SSRlocalStoragewithout checking ifwindowis definedPageTabs hydration mismatch
The
PageTabscomponent used a global counterrenderCountForTocUpdatethat was incremented on every render. This counter was used to conditionally render a hidden<h2>element for TOC updates. Since the counter value could differ between server and client renders, this caused hydration mismatches.Test plan
groupIdwork correctly (state persisted in localStorage)