feat(client): expose host React to Module Federation remotes - #2828
Conversation
856013d to
91dba70
Compare
Nsttt
left a comment
There was a problem hiding this comment.
This will hep us a lot to leverage https://github.com/module-federation/vinext/
commit: |
Performance benchmarksCompared 0 improved · 0 regressed · 6 within ±1.5%
View detailed results and traces 🟢 improvement · 🔴 regression · ⚫ change below 1.5% · paired base/head |
|
/bigbonk review for issues |
There was a problem hiding this comment.
Reviewed the Module Federation client React bridge. The implementation is clean, well-scoped, and nicely tested. "First-registration-wins" plus the typeof window === "undefined" server guard is the right shape, and the ordering tests for both routers give good coverage. A couple of questions/nits below — none are blockers.
Verified:
dist/client/index.jswill be emitted (vite.config.tsusesentry: ["src/**/*.ts"]withunbundle: true), so the new./clientexport path resolves.- The server-side guard prevents accidental RSC/SSR global registration, and the bootstrap is only imported from client entries, so the side effect runs in the browser as intended.
- Ordering is guaranteed by textual import position (ESM evaluates static imports top-to-bottom), matching what the ordering tests assert.
Main question: the "use client" directive on react-instance-bootstrap.ts — see inline comment. I don't think it's harmful, but it may be unnecessary and slightly misleading.
| @@ -0,0 +1,6 @@ | |||
| "use client"; | |||
There was a problem hiding this comment.
Is the "use client" directive actually needed here? This module is only imported directly by the browser/client entries (app-browser-entry.ts, pages-client-entry.ts, pages-dev-hydration.ts), which are already client-side. It's never consumed as a client-reference boundary from the RSC graph.
Unlike the shims in shims/*.tsx that use "use client" (which are imported across the RSC boundary and need the plugin to generate client-reference proxies), this is a pure side-effect module with no exports. Adding "use client" makes @vitejs/plugin-rsc treat it as a client boundary unnecessarily, and it's inconsistent with the other files in src/client/ (e.g. navigation-runtime.ts) which don't carry the directive.
If there's a concrete reason it's required (e.g. a specific env where the bootstrap gets pulled through the RSC transform), a one-line comment explaining it would help. Otherwise consider dropping it.
| * Server and RSC environments always keep their condition-specific React | ||
| * instance local. | ||
| */ | ||
| export function getVinextReact(reactInstance: typeof import("react")): typeof import("react") { |
There was a problem hiding this comment.
Minor robustness note (non-blocking): getVinextReact implements a first-writer-wins singleton keyed on Symbol.for("vinext.client.react"). If a statically-hoisted or dynamically-imported remote ever evaluates and calls getVinextReact(remoteReact) before the host bootstrap runs, the remote's React becomes the registered singleton — the opposite of the intent documented in the README ("vinext registers the host's browser React instance before application modules execute").
In practice the ordering holds because the bootstrap import is textually first in every client entry, so this is fine for the supported cases. Worth keeping in mind if App Router MF SSR / earlier remote evaluation lands later — the "host always wins" guarantee is really "whoever registers first wins", which happens to be the host today.
|
Review posted successfully to PR #2828. SummaryI reviewed the Module Federation client React bridge PR. Overall it's a clean, well-scoped, well-tested change. I posted a review with 2 non-blocking inline comments: Verified as correct:
Feedback raised:
Neither is a blocker; the PR is in good shape. |
Summary
vinext/cliententry point withgetVinextReact()so Module Federation remotes can reuse the host React instanceWhy
Module Federation remotes can evaluate with a different React module instance from the vinext host. Components that read hooks from that copy can hit React dispatcher mismatches and fail with invalid hook call errors.
The client bridge stores the first browser React instance under a global symbol and returns it to remotes explicitly. First-registration semantics also keep the host instance stable when duplicate modules are evaluated or HMR reloads the bridge. Server and RSC environments continue to use their condition-specific React instances without global registration.
User impact
Client-side Module Federation integrations can import
getVinextReactfromvinext/clientand resolve hooks against the vinext host React instance. Consumers still need compatible singletonreact,react/,react-dom, andreact-dom/sharing configuration. This does not add App Router Module Federation SSR or rewrite React imports in third-party packages.Validation
pnpm test tests/client-react-bridge.test.ts tests/entry-templates.test.ts tests/deploy.test.ts— 369 tests passedpnpm run check— formatting, lint, type checks, Next.js type sync, and public shim checks passed