fix(runtime-core): guard targetShared.get before calling in loadShare#4655
fix(runtime-core): guard targetShared.get before calling in loadShare#4655knappam wants to merge 3 commits intomodule-federation:mainfrom
Conversation
✅ Deploy Preview for module-federation-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
🦋 Changeset detectedLatest commit: ae100e9 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e845ced2d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (typeof targetShared.get !== 'function') { | ||
| return (() => ({})) as unknown as () => T; | ||
| } |
There was a problem hiding this comment.
Return no-match sentinel when shared factory is missing
When targetShared.get is absent, this branch now returns a dummy factory (() => ({})) instead of signaling that no usable shared module was found. That changes loadShare from a miss to a success and breaks callers that rely on the documented false sentinel for fallback behavior (for example packages/webpack-bundler-runtime/src/consumes.ts checks factory === false before using the local getter). In the malformed-share-scope scenario handled by this patch, consumers can now silently receive an empty module object rather than taking fallback logic, causing missing-export failures downstream.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This is a valid point, will update
When the host's share scope contains a dependency that a remote does not declare, the share scope entry has no get() factory. The loadShare method calls targetShared.get!() without checking, crashing with 'targetShared.get is not a function'. Add a typeof check before both call sites in SharedHandler.loadShare. If get is not a function, return an empty factory instead of crashing. Fixes module-federation#2497
2e845ce to
1552989
Compare
|
@zackarychapple please help check metro part |
Description
When the host's share scope contains a dependency that a remote does not
declare, the share scope entry has no
get()factory.SharedHandler.loadSharecalls
targetShared.get!()without checking, crashing withtargetShared.get is not a function(orM.get is not a functioninminified builds).
This is particularly common in Metro MF on React Native/Android, where the
host app shares a dependency (e.g.
moment) that a mini app remote does notdeclare. During share scope negotiation the runtime encounters the entry
without a
getfactory and crashes.The fix adds a
typeof targetShared.get !== 'function'guard before bothcall sites in
SharedHandler.loadShare. Ifgetis not a function, theruntime returns an empty factory instead of crashing.
Confirmed on a real Android device:
M.get is not a function (it is undefined)when loading the remoteRelated Issue
Fixes #2497
Types of changes
Checklist