MSD: Refactor interim omnibar to hooks-based data flow by StevenDufresne · Pull Request #109881 · Automattic/wp-calypso

StevenDufresne · 2026-04-08T04:51:49Z

Proposed Changes

Refactor loadOmnibar to mount a hooks-based container (useInterimOmnibarData) instead of driving React imperatively with root.render and QueryObserver.subscribe after hydrateRoot.

Why are these changes being made?

The previous implementation raced hydration and caused React to bail with:

This root received an early update, before anything was able to hydrate. Switched the entire root to client rendering.

With the refactor, every post-hydration update flows through normal React rendering — there is no external code path that can update the root between hydrateRoot and its commit.

Testing Instructions

Enable the dashboard/omnibar feature flag.
Hard-reload a Dashboard page with the omnibar visible.
Confirm the "This root received an early update…" warning no longer appears in the console.
Confirm the omnibar still shows the current user and the active site, and re-renders when switching sites.
Confirm the mobile menu and notifications toggles still work.

Pre-merge Checklist

Has the general commit checklist been followed? (PCYsg-hS-p2)
Have you written new tests for your changes?
Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
Have you checked for TypeScript, React or other console errors?
Have you tested accessibility for your changes? Ensure the feature remains usable with various user agents (e.g., browsers), interfaces (e.g., keyboard navigation), and assistive technologies (e.g., screen readers) (PCYsg-S3g-p2).
Have you used memoizing on expensive computations?
Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-aUh-p2)?

github-actions · 2026-04-08T04:52:02Z

Calypso Live (direct link)

https://calypso.live?image=registry.a8c.com/calypso/app:build-175197

Jetpack Cloud Live (direct link)

https://calypso.live?image=registry.a8c.com/calypso/app:build-175197&env=jetpack

Automattic for Agencies Live (direct link)

https://calypso.live?image=registry.a8c.com/calypso/app:build-175197&env=a8c-for-agencies

Dashboard Live (dotcom) (direct link)

https://calypso.live?image=registry.a8c.com/calypso/app:build-175197&env=dashboard

Dashboard Live (CIAB) (direct link)

https://calypso.live?image=registry.a8c.com/calypso/app:build-175197&env=dashboard-ciab

client/dashboard/app/interim-omnibar/test/use-interim-omnibar-data.test.tsx

fushar

Other than that I believe this is good 👍 Tested.

client/dashboard/app/interim-omnibar/test/use-interim-omnibar-data.test.tsx

p-jackson

I thought this might produce errors because the <InterimOmnibar> rendered on the server at client/document/index.jsx doesn't have the container and the client-side one does. I thought that's the sort of thing that breaks hydration.
Evidently not.

A nit is that I would perhaps combine the container and hook into a single tsx file, since they really only work together.

p-jackson · 2026-04-09T02:29:56Z

client/dashboard/app/interim-omnibar/use-interim-omnibar-data.ts

+
+	const { data: site = null } = useQuery( {
+		...siteByIdQuery( siteId ?? 0 ),
+		enabled: hydrated && !! siteId,


I think we need to add a isRecentSitesLoaded check here too. Otherwise the omnibar could flash the primary blog details before switching to the recent site details.

The problem is that the siteByIdQuery is going to start up as soon as there is any site ID available, but we really want it to wait for the preferences to finish loading first.

I was meaning to fix this behavior in my next PR 😄 it's related to the origin_site_id check, where it trumps everything.

Yeah definitely related to that!

p-jackson · 2026-04-09T02:35:21Z

client/dashboard/app/interim-omnibar/use-interim-omnibar-data.ts

+			currentRoute: window.location.pathname,
+		};


We should return the callbacks in this case too. It's a small thing, but the <InterimOmnibar> rebuilds the redux store whenever this callback changes, so we don't need these callbacks to unnecessarily change.

Suggested change

currentRoute: window.location.pathname,

};

currentRoute: window.location.pathname,

onToggleMenu,

onToggleNotifications,

};

Which means we should fix up the hook's jsdoc too.

I don't understand, why didn't we also do this (returning callbacks pre-hydration) before this PR?

In the .render() call was always passing the callback props. The hydrateRoot call wasn't, but that was my mistake. I suspect that between hydrateRoot and .render is was probably tearing down all the elements any, and so it was re-creating a store then no matter what.

client/dashboard/app/interim-omnibar/test/use-interim-omnibar-data.test.tsx

The previous implementation drove React imperatively via `root.render` and `QueryObserver.subscribe` after `hydrateRoot`, which raced hydration and caused React to bail out with "This root received an early update, before anything was able to hydrate. Switched the entire root to client rendering." Move the data fetching into a new `InterimOmnibarContainer` that uses TanStack Query hooks (`useQuery` for auth, recentSites, and the active site). A `useState(false)` + `useEffect` gate ensures the first render mirrors the SSR output exactly (`user = initialUser`, `site = null`, no toggle callbacks); after hydration commits, the container switches to hook-driven data and wires up the toggle handlers. All post-hydration updates now flow through normal React rendering instead of external imperative calls. `loadOmnibar` shrinks to just click delegation and a single `hydrateRoot` call. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Extract the data-flow logic into an exported `useInterimOmnibarData` hook so it can be tested in isolation without rendering the full masterbar tree. Add unit tests covering: bootstrapped user via initialData, toggle callback wiring, recent-site loading, primary_blog fallback, and the null-site state when no siteId is known. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Replace the weak assertions (`initialData` pass-through and the `site === null` default) with a `renderToStaticMarkup`-based test that verifies the hook's pre-hydration output matches the SSR shape exactly (user, null site, no callbacks). This is the contract hydration actually depends on. Drop the "keeps site null" test — it only asserted a destructuring default and would have passed even if the enabled guard was removed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Move the hook out of `interim-omnibar-container.tsx` into `use-interim-omnibar-data.ts`, matching the source-file naming convention used by `hooks/use-persistent-view.ts` (and lining up with the test file name). Tests now build a fresh `OmnibarEvents` per test instead of subscribing to the shared module singleton, so a failed assertion can't leak a dangling subscription into later tests. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

This reverts commit 84a0480.

Avoids nesting a second provider inside InterimOmnibar's own.

p-jackson

I've tested it with wpcom-bootstrap-user too to make sure there's no errors still.

Finally no race conditions in our hydration I think 🤞🤞🤞🤞

fushar

👍

fushar · 2026-04-09T03:41:13Z

client/dashboard/app/interim-omnibar/interim-omnibar-container.tsx

+	const { data: site = null } = useQuery(
+		{
+			...siteByIdQuery( siteId ?? 0 ),
+			enabled: hydrated && !! siteId && ! isRecentSitesLoading,


It seems this means the queries can't run in parallel. In any case, I'll try to fix it up later.

Yes Claude was mad at me for breaking the parallel requests. But I think following it up once we have the final ordering of site IDs makes sense.

fushar · 2026-04-09T03:50:43Z

🎉

StevenDufresne mentioned this pull request Apr 8, 2026

MSD: Load locale data for the interim omnibar #109882

Closed

8 tasks

StevenDufresne marked this pull request as ready for review April 8, 2026 05:55

StevenDufresne requested a review from a team as a code owner April 8, 2026 05:55

matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Apr 8, 2026

claude bot reviewed Apr 8, 2026

View reviewed changes

client/dashboard/app/interim-omnibar/test/use-interim-omnibar-data.test.tsx Outdated Show resolved Hide resolved

StevenDufresne mentioned this pull request Apr 8, 2026

MSD: Server-render the interim omnibar in the user's language #109883

Closed

8 tasks

fushar approved these changes Apr 9, 2026

View reviewed changes

client/dashboard/app/interim-omnibar/test/use-interim-omnibar-data.test.tsx Outdated Show resolved Hide resolved

StevenDufresne force-pushed the refactor/interim-omnibar-hooks-data-flow branch from 97c98cf to b03137c Compare April 9, 2026 02:44

p-jackson reviewed Apr 9, 2026

View reviewed changes

p-jackson force-pushed the refactor/interim-omnibar-hooks-data-flow branch from b03137c to bf4d95a Compare April 9, 2026 03:26

StevenDufresne and others added 11 commits April 9, 2026 15:26

MSD: Trim historical comments from interim omnibar container

5964fcd

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Revert "Temporary turn on omnibar."

99c0c0d

This reverts commit 84a0480.

This PR makes no .json config changes

4bed443

Do not fetch omnibar site till preferences have loaded

3d21634

useInterimOmnibarData always returns callback props

55cd6ee

Combine container and hook into one tsx module

0cb48fe

Pass queryClient to useQuery, drop QueryClientProvider

9cf950d

Avoids nesting a second provider inside InterimOmnibar's own.

p-jackson force-pushed the refactor/interim-omnibar-hooks-data-flow branch from bf4d95a to 9cf950d Compare April 9, 2026 03:27

Fix comment

1f4ab13

p-jackson approved these changes Apr 9, 2026

View reviewed changes

fushar reviewed Apr 9, 2026

View reviewed changes

p-jackson merged commit 9c43891 into trunk Apr 9, 2026
15 checks passed

p-jackson deleted the refactor/interim-omnibar-hooks-data-flow branch April 9, 2026 03:47

github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Apr 9, 2026

Conversation

StevenDufresne commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed Changes

Why are these changes being made?

Testing Instructions

Pre-merge Checklist

Uh oh!

github-actions bot commented Apr 8, 2026 • edited by matticbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

fushar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

p-jackson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

p-jackson left a comment

Choose a reason for hiding this comment

Uh oh!

fushar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fushar commented Apr 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

StevenDufresne commented Apr 8, 2026 •

edited

Loading

github-actions bot commented Apr 8, 2026 •

edited by matticbot

Loading