MSD: Server-render the interim omnibar in the user's language#109883
Closed
StevenDufresne wants to merge 17 commits intotrunkfrom
Closed
MSD: Server-render the interim omnibar in the user's language#109883StevenDufresne wants to merge 17 commits intotrunkfrom
StevenDufresne wants to merge 17 commits intotrunkfrom
Conversation
Jetpack Cloud Live (direct link)
Automattic for Agencies Live (direct link)
Dashboard Live (dotcom) (direct link)
|
Contributor
|
This PR modifies the release build for the following Calypso Apps: For info about this notification, see here: PCYsg-OT6-p2
To test WordPress.com changes, run |
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.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
97c98cf to
b03137c
Compare
Add a `loadDashboardLocaleData` Express middleware that, after `setUpRoute` has populated `req.context.lang` from the bootstrapped user, reads the matching locale JSON from `public/languages/` and applies it to the global `defaultI18n` singleton from `@wordpress/i18n`. The data is also stashed on `req.context.localeData` and inlined into the HTML response as `var dashboardLocaleData = ...`. The interim omnibar's `loadOmnibar` reads that global before calling `hydrateRoot` and applies it to `defaultI18n`, so the client's first render uses the same translated strings as the server — no hydration mismatch and no flash of English. Wired into the dashboard section paths (dotcom, ciab, start-store) in `client/server/pages/index.js` via a new optional `extraMiddleware` parameter on `handleSectionPath`. Untested: this path requires `wpcom-user-bootstrap: true` so the server has user identity at render time. Local dev (with `dashboard-development.json` defaults) cannot reach this code path end-to-end. Verification will need a bootstrapped staging environment. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1d459c5 to
c82949c
Compare
bf4d95a to
9cf950d
Compare
Rework the SSR-translated interim omnibar path so the HTML payload is not bloated with inlined locale data. Instead, the client awaits the locale fetch before hydrating and shares a module-level promise cache with the main dashboard `I18nProvider`, so there is exactly one network request per page load (and zero on repeat visits once the CDN response is cached by the browser). Extracts `getUserLanguage` and `loadUserLocale` into `client/dashboard/app/shared-locale-loader.ts`. Both `loadOmnibar` and `I18nProvider` use them, consolidating the language fallback chain and deduplicating the fetch. The server middleware still populates `defaultI18n` for the SSR render so the masterbar HTML ships with translated strings; it no longer stashes the data on `req.context.localeData`. The client `loadOmnibar` now blocks on `loadUserLocale(window.currentUser)` before calling `hydrateRoot` so the first client render matches the SSR-translated HTML byte-for-byte. Requires `wpcom-user-bootstrap: true` so the server has the user's locale at render time. Dashboard envs (horizon/stage/production) have this enabled; dev environments do not by default. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Closes a subtle regression from the shared-loader refactor: the old `fetchLocaleData` path called `resetLocaleData` for the English branch, which cleared any previously-loaded locale. `loadUserLocale` was returning early without resetting, so switching from a non-English locale to English mid-session would leave stale translations in `defaultI18n`. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
`setLocaleData` merges into any existing locale data, so switching languages in the same session (e.g. fr → es) would leave stale fr keys in place alongside the new es keys. Use `resetLocaleData( data )` instead, which clears and replaces. Track the currently-applied language in a module variable so re-awaits of a cached data promise re-apply the data if the singleton has been clobbered by a different language in between. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
No behavior change. Typed as an Express `RequestHandler`, with a `CalypsoRequest` alias that narrows the request's `context.lang` extension. The `localeDataCache` Map is now generically typed, and the fetch/parse chain is typed as `LocaleData` throughout. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fixes a hydration mismatch for users with locale variants or partially-translated languages. The client's `getUserLanguage` was returning `localeVariant || localeSlug || ...`, but the server's `setUpLoggedInRoute` uses `localeSlug` (the base) and gates on `use_fallback_for_incomplete_languages` + `isTranslatedIncompletely`. For Spanish-Mexico users (`localeSlug: "es"`, `localeVariant: "es-mx"`) the client was fetching `es-mx-v1.1.json` while the server had loaded `es-v1.1.json`, producing different strings and a hydration mismatch. For partial-with-fallback users (e.g. Quechua with fallback on) the server rendered English while the client fetched the partial bundle, also mismatching. The fix mirrors the server's derivation exactly: use `localeSlug` as the returned slug, use `localeVariant || localeSlug` for the incompleteness check, and fall back to English when the gate triggers. Falls through to `locale_variant` / `language` for non-bootstrapped sessions (where there is no SSR content to match against anyway). While touching `loadOmnibar`, parallelize the dynamic import of the omnibar chunk with the locale fetch via `Promise.all` — saves up to a few hundred ms on first visit when both run cold. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Two related fixes in the loadUserLocale catch branch: 1. Callers treat an `undefined` return as an English fallback and update the DOM accordingly. But the previous catch only returned `undefined` — it did not touch `defaultI18n`. So after a successful French load followed by a failed Spanish load, defaultI18n would still contain French data while the DOM claimed to be English, rendering mixed-language UI. 2. The rejected fetch promise was cached in dataPromises forever, so a later call for the same language would return the cached rejection without retrying. Transient CDN failures became permanent until a full page reload. Fix both by resetting defaultI18n to English state and deleting the failed promise from the cache. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…eware Use `resetLocaleData` instead of `setLocaleData` so each request fully replaces the locale data rather than merging into whatever the previous request left behind. The server's `defaultI18n` is a process-global that persists between requests, so under mixed-locale traffic the previous behavior would leave stale keys from the prior request alongside the current locale — a mixed-language SSR render for whichever keys were missing from the current bundle. This mirrors the fix already landed on the client side in 5ea26cb ("Use resetLocaleData in loadUserLocale so switches replace cleanly"). The same bug was present on both sides; I caught only the client one initially. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Fixes a race condition where a slow fetch completing after the caller
had moved on to a different language would overwrite the newer locale
data in `defaultI18n`, leaving the singleton in French while the DOM
and React state said Spanish.
The root cause was that `loadUserLocale` owned both the cached fetch
(module-level, shared) and the `defaultI18n.resetLocaleData` side
effect. The `.then` inside the loader ran unconditionally on resolve,
guarded only by a module-level `appliedLanguage` tracker that couldn't
know whether the original caller was still interested.
Split the responsibilities:
- `loadUserLocaleData` — pure cache-aware fetch. Returns the raw
LocaleData or undefined for English/error. No singleton mutation,
no shadow-state tracker.
- Callers (`loadOmnibar`, `I18nProvider`) now call `resetLocaleData`
themselves after awaiting the data, gated on their own
cancellation state. The `I18nProvider`'s existing `cancelled`
flag now correctly prevents stale completions from overriding
fresher ones.
Also eliminates `appliedLanguage` and the three shotgun-surgery sites
that updated it. Pure fetch is also easier to test.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Both `loadOmnibar` and the dashboard `I18nProvider` had an identical four-line if/else that called `resetLocaleData(data)` for the success branch and `resetLocaleData()` for the undefined branch. The two branches are equivalent: `resetLocaleData` accepts an optional `LocaleData` argument and passing `undefined` has the same effect as passing nothing (both clear the data bag and reapply default-domain metadata). Collapse to a single `resetLocaleData( data )` call at each site. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
c0179bd to
3c4021c
Compare
Base automatically changed from
refactor/interim-omnibar-hooks-data-flow
to
trunk
April 9, 2026 03:47
Contributor
Author
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.
Part of DOTMSD-1199
Stacked on #109881
Proposed Changes
loadDashboardLocaleDataExpress middleware loads the user's locale JSON frompublic/languages/(falling back to the Calypso CDN) and applies it to thedefaultI18nsingleton for the duration of the request's render, so the server-side render of the interim omnibar emits translated strings.extraMiddlewareparameter onhandleSectionPath.shared-locale-loader.tswithgetUserLanguage(language fallback chain) andloadUserLocale(module-level promise cache that applies the locale todefaultI18n). Used by bothloadOmnibarand the main dashboardI18nProvider, so a given language is fetched once per page load — and zero times on repeat visits once the CDN response is browser-cached.loadOmnibarnow awaitsloadUserLocalebefore callinghydrateRoot, so the first client render matches the SSR-translated HTML and hydration succeeds without a mismatch or an English flash.client/dashboard/app/i18n.tsxrefactored to use the shared loader — removes the duplicatefetchLocaleDatadefinition.Why are these changes being made?
Support non-english languages during server-side rendering.
Requirements & caveats
Requires
wpcom-user-bootstrap: trueThis fix depends on the server knowing the user's locale at render time, which requires
wpcom-user-bootstrapto be enabled sosetUpLoggedInRoutecan populatereq.context.userandreq.context.lang.wpcom-user-bootstrapdashboard-productiondashboard-stagedashboard-horizondashboard-developmentDev environments (
dashboard-development.json) have bootstrap off by default, soreq.context.langstays unset, the middleware short-circuits to English, and the flash returns. To test locally, developers need to enablewpcom-user-bootstrap: truein their dev config and have thewpcom_calypso_rest_api_keysecret available. Non-English testers running on default dev config will continue to see the flash until the environment is set up for bootstrap.First-visit latency
loadOmnibarblocks hydration on the locale fetch. On a cold browser cache, the fetch takes ~100-900ms depending on network (thefr-v1.1.jsonbundle is ~900 KB gzipped). Repeat visits hit the browser cache and resolve near-instant. The main dashboard has always paid this cost; we're now waiting on the same fetch the dashboard's ownI18nProvideralready issued.Partially-translated locales
Users with
use_fallback_for_incomplete_languagesenabled and a locale below the completeness threshold get English SSR (handled bysetUpLoggedInRouteupstream —req.context.langis not set, our middleware short-circuits). Correct behavior; worth flagging because it's not immediately obvious.Shared-loader unmount handling
client/dashboard/app/i18n.tsxpreviously used anAbortControllerto cancel the in-flight locale fetch if the component unmounted mid-fetch (added in response to #103635 review). The refactor replaces that with acancelledref-style flag in theuseEffectcleanup — which was the other option suggested in the original review comment.The trade-off: the
cancelledflag still preventssetStateon an unmounted component (the original correctness concern), but the HTTP request is no longer aborted when one caller unmounts, because the promise is now shared withloadOmnibar. In practice the fetch completes and warms the browser cache for the next visit, so this is effectively a small net positive on repeat-visit latency. The component rarely unmounts mid-fetch anyway (the dashboardI18nProviderstays mounted for the session).HTML payload
No regression. The middleware does not inline locale data into the HTML response. An earlier iteration of this PR tried inlining and measured a ~867 KB gzipped per-request overhead — unacceptable. The current approach uses the existing CDN URL (
widgets.wp.com/languages/calypso/{lang}-v1.1.json) with its far-future cache headers instead.Testing Instructions
Local dev (requires bootstrap setup):
wpcom-user-bootstrap: trueinconfig/dashboard-development.jsonand ensurewpcom_calypso_rest_api_keyis available in secrets.window.currentUser. It should be defined.http://my.localhost:3000/sites) — confirm the omnibar shows translated strings on the first visible frame with no English flash.Via calypso.live (Dashboard Live, dotcom):
https://calypso.live?image=registry.a8c.com/calypso/app:commit-<sha>&env=dashboardfrom the PR comment.wpcom-user-bootstrapis disabled at boot and this path won't fire. Testing here will still show the flash — deploy to horizon or production to verify the fix in action.Language switching (same session):
Pre-merge Checklist