-
-
Notifications
You must be signed in to change notification settings - Fork 358
fix(nextjs-mf): Refine Module Federation shares and correct request keys #3791
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
base: share-filter
Are you sure you want to change the base?
Conversation
# Conflicts: # pnpm-lock.yaml
# Conflicts: # pnpm-lock.yaml
# Conflicts: # pnpm-lock.yaml
# Conflicts: # pnpm-lock.yaml
# Conflicts: # pnpm-lock.yaml
…nd add cleanup step
# Conflicts: # apps/3000-home/package.json # package.json # packages/sdk/src/node.ts # pnpm-lock.yaml
…em and refactor related logic
…odulesReconstructedLookup option
This commit addresses feedback on the previous module federation sharing
updates:
1. Corrects an issue where `$` was used at the end of `request` strings
in shared module configurations (e.g., `unfetch$`). This suffix is
not standard for Module Federation share keys and has been removed.
The `import` paths for these modules remain correct.
2. Reinstates prefix-based shares in `share-internals-client.ts` for
`next/dist/shared/` (with `include: /shared-runtime/`) and
`next/` (with `exclude: /dist/`). This aligns with the preference
for using broader shares where they suffice.
3. Refines explicit shares in `share-internals-client.ts`:
- Removed the explicit share for `next/dist/shared/lib/shared-runtime.js`
as it is now covered by the reinstated prefix share.
- Retained necessary explicit shares for React, Server Action client
modules, polyfills mapping to Next.js internals, and core Next.js
client utilities that map user-facing APIs (e.g., `next/link`) to
specific internal `next/dist/...` paths.
4. Reviewed explicit shares in `share-internals-server.ts` and confirmed
their necessity due to specific internal `import` paths (for vendored
React, Server Actions, aliased modules like `@vercel/og`, and core
server APIs like `next/headers`) and the absence of broad prefix
shares on the server side. No shares were removed from this file.
This approach ensures that module sharing is both comprehensive for
critical Next.js internals and cleaner by preferring prefix shares where
appropriate, while also correcting the format of request keys.
|
This commit applies several corrections to the module federation
sharing configurations in `share-internals-client.ts` and
`share-internals-server.ts` based on recent feedback:
1. **Removed Polyfill Sharing**:
- All shared configurations for polyfills (e.g., `unfetch`,
`isomorphic-unfetch`, `whatwg-fetch`, `object-assign`, `url`)
have been removed from `share-internals-client.ts`.
- Confirmed no such polyfill shares existed in
`share-internals-server.ts`.
2. **Restored Deterministic Key Generation**:
- Reverted the key generation logic for shared module configurations
in both `share-internals-client.ts` and `share-internals-server.ts`
to be deterministic (based on shareKey, layer, and index),
removing any `Math.random()` components.
3. **Verified `nodeModulesReconstructedLookup`**:
- Confirmed that React module configurations in `getReactGroupServer`
within `share-internals-server.ts` correctly maintain
`nodeModulesReconstructedLookup: false`, consistent with their
previous state in the analyzed file version.
These changes ensure a more precise and deterministic approach to
sharing Next.js internals for Module Federation.
60d6549 to
055deac
Compare
3c3d92d to
feb05f0
Compare
5011aff to
aab5c25
Compare
This commit addresses feedback on the previous module federation sharing updates:
Corrects an issue where
$was used at the end ofrequeststrings in shared module configurations (e.g.,unfetch$). This suffix is not standard for Module Federation share keys and has been removed. Theimportpaths for these modules remain correct.Reinstates prefix-based shares in
share-internals-client.tsfornext/dist/shared/(withinclude: /shared-runtime/) andnext/(withexclude: /dist/). This aligns with the preference for using broader shares where they suffice.Refines explicit shares in
share-internals-client.ts:next/dist/shared/lib/shared-runtime.jsas it is now covered by the reinstated prefix share.next/link) to specific internalnext/dist/...paths.Reviewed explicit shares in
share-internals-server.tsand confirmed their necessity due to specific internalimportpaths (for vendored React, Server Actions, aliased modules like@vercel/og, and core server APIs likenext/headers) and the absence of broad prefix shares on the server side. No shares were removed from this file.This approach ensures that module sharing is both comprehensive for critical Next.js internals and cleaner by preferring prefix shares where appropriate, while also correcting the format of request keys.
Description
Related Issue
Types of changes
Checklist