fix(cls): reduce CLS via shell alignment + synchronous React mount + web-vitals instrumentation#559
fix(cls): reduce CLS via shell alignment + synchronous React mount + web-vitals instrumentation#559aafre wants to merge 3 commits into
Conversation
Move the .shell-header / .shell-mobile-ad / route-aware CSS block from inside #root to <head> so React cannot remove it during the initial mount, eliminating the intermediate-state style collapse. Remove the unconditional shell-mobile-ad placeholder (282 px on mobile). VITE_ENABLE_EXPLICIT_ADS is false in PROD so React never renders the mobile-top slot; the shell reservation was causing a 282 px shift on every mobile page load.
Use flushSync() on the initial createRoot().render() so React's first paint commits synchronously, eliminating the intermediate DOM state (shell-header gone, H1 still visible) that caused the 64 px CLS shift measured at ~181 ms. Install web-vitals@5 and wire onCLS with attribution logging to console. Wire to PostHog when that integration ships.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request addresses Cumulative Layout Shift (CLS) issues by moving the shell layout CSS to the <head> in index.html and introducing web-vitals to track CLS metrics. It also attempts to use flushSync to force a synchronous initial render. Feedback recommends removing unused .shell-mobile-ad CSS rules from index.html and removing the flushSync wrapper around createRoot().render(), as it is unsupported in React 18, triggers console warnings, and is redundant now that the CSS has been moved to the <head>.
| <style> | ||
| .shell-header{height:64px} | ||
| @media(min-width:640px){.shell-header{height:72px}} | ||
| .shell-mobile-ad{display:block} | ||
| @media(min-width:768px){.shell-mobile-ad{display:none}} | ||
| .not-home #shell-landing{display:none} | ||
| #shell-generic{display:none} | ||
| .not-home #shell-generic{display:block} | ||
| </style> |
There was a problem hiding this comment.
Since the .shell-mobile-ad placeholder element was removed from the HTML shell, the .shell-mobile-ad CSS rules in the <style> block are now unused. Removing them keeps the HTML shell clean and avoids dead code.
<style>
.shell-header{height:64px}
@media(min-width:640px){.shell-header{height:72px}}
.not-home #shell-landing{display:none}
#shell-generic{display:none}
.not-home #shell-generic{display:block}
</style>There was a problem hiding this comment.
@gemini-code-assist Good catch — fixed in 0a6facb. Removed the two orphaned .shell-mobile-ad rules from the <head> style block.
There was a problem hiding this comment.
Great work on the fix, aafre. The removal of the orphaned CSS rules looks correct and maintains the cleanliness of the shell. No further action is needed on this thread.
| // flushSync forces the initial React render to be synchronous, eliminating | ||
| // the intermediate DOM state (shell-header gone, H1 still visible) that | ||
| // causes the 64px CLS shift measured by Codex at ~181ms. | ||
| flushSync(() => { | ||
| createRoot(document.getElementById("root")!).render( | ||
| <StrictMode> | ||
| <HelmetProvider> | ||
| <App /> | ||
| </HelmetProvider> | ||
| </StrictMode> | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Wrapping createRoot().render() inside flushSync is not supported in React 18 and will trigger a console warning: Warning: flushSync was called manually and some updates were not flushed. This is a bug in React.
Since you have already moved the <style> tag to the <head> (which successfully prevents the header height from collapsing during the mount phase), the primary cause of the 64px CLS shift is resolved. Therefore, flushSync is no longer necessary and should be removed to avoid React runtime warnings and potential compatibility issues with concurrent rendering features.
createRoot(document.getElementById("root")!).render(
<StrictMode>
<HelmetProvider>
<App />
</HelmetProvider>
</StrictMode>
);
There was a problem hiding this comment.
@gemini-code-assist Verified: no React warning is produced in practice. After reloading the page, the browser console shows zero React warnings — only the expected Supabase dev-env notice. flushSync around an initial createRoot().render() outside any React context does not trigger the "some updates were not flushed" warning in React 18.3.1.
Keeping flushSync as belt-and-suspenders: the CSS-to-<head> change prevents style collapse, but flushSync eliminates any intermediate DOM state entirely by making the first commit synchronous — a stronger guarantee at zero runtime cost. Both are needed to cover all intermediate-state CLS paths identified in the Codex forensic audit. Lab CLS is 0.00 on mobile and tablet with both changes in place.
There was a problem hiding this comment.
Thank you for the clarification, @aafre. Given that you have verified the behavior in your specific environment and the lab results confirm the elimination of CLS, the use of flushSync here serves as a deliberate architectural choice to ensure synchronous commitment of the initial render. While generally discouraged in React 18 due to potential conflicts with concurrent features, its application at the root entry point to force a synchronous first paint is a known pattern for addressing specific layout shift issues during hydration. Since this is isolated to the initial mount and you have confirmed it does not trigger warnings in your build, it is acceptable in this context.
The .shell-mobile-ad placeholder div was removed in the previous commit; remove the now-unused display rules from the <head> style block.
1 participant
Summary
Reduces lab CLS from 0.11 → 0.00 (measured at mobile 375×667 with 4× CPU throttle and at tablet 768×1024) by addressing the two root causes identified in the Codex forensic audit, plus adds web-vitals attribution logging for field debugging.
Three changes, two commits:
Commit 1 — HTML shell alignment (
index.html):.shell-header/.shell-mobile-ad/ route-aware CSS block from inside#rootto<head>. When React callscreateRoot().render()and wipes#root's children, the<style>tag was removed first, collapsing.shell-headerheight to 0 while#shell-landingcontent was still visible — a 64 px upward jump. CSS in<head>cannot be removed by React.shell-mobile-adplaceholder (reserves 250 px + 32 px margin = 282 px).VITE_ENABLE_EXPLICIT_ADSisfalsein PROD so React never renders the mobile-top ad slot; the shell was reserving space React always removed, causing a 282 px shift on every mobile load.Commit 2 — JS entry point (
main.tsx+package.json):3. Wrap the initial
createRoot().render()influshSync()so React's first render commits synchronously, eliminating any intermediate DOM state (shell-header partially removed, H1 still visible) that could cause the 64 px shift.4. Install
web-vitals@5and wireonCLSwith attribution to console. LogslargestShiftTarget,largestShiftTime, andloadStatefor field debugging. Ready to wire to PostHog when that integration ships.Changes
resume-builder-ui/index.html<head>; removeshell-mobile-adplaceholderresume-builder-ui/src/main.tsxflushSyncon initial render; addonCLSweb-vitals loggingresume-builder-ui/package.jsonweb-vitals@^5.3.0Test plan
npx vitest run— 1445 tests pass, 0 failuresnpx eslint src/main.tsx --max-warnings 0)test-screenshots/after-fix-mobile-375.pngandafter-fix-tablet-768.pngBefore / After
Risk: MEDIUM
Touches
index.htmlandmain.tsx— the two entry points.flushSyncis an intentional React escape hatch for this use case (forcing synchronous initial mount); it does not affect subsequent re-renders or Concurrent Mode features. Rollback =git revert HEAD~1orgit revert HEAD.Note — 48h attribution gap: Do not merge until ≥48h after PR #556 (VideoObject schema removal) merges, per SEO attribution policy.
Related
C:\Users\Amit\.claude\plans\stg-backlog-context.md