fix(runtime-core): resolve remote runtime instance identity explicitly during removal - #5029
fix(runtime-core): resolve remote runtime instance identity explicitly during removal#5029ScriptedAlchemy wants to merge 2 commits into
Conversation
…when cleanup misses removeRemote (used by registerRemotes with force: true) looked up the remote's runtime instance in __FEDERATION__.__INSTANCES__ only by the registered name. When that differs from the name the remote was built with, the lookup silently missed: the container global and module cache were dropped but the old instance stayed registered, so every forced re-registration leaked a full container graph. The lookup now falls back to matching by entryGlobalName (the runtime instance name for enhanced/webpack-built containers) and logs a warning when no instance can be found instead of failing silently. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 46e8265 The changes in this PR will be included in the next version bump. This PR includes changesets to release 48 packages
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 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
@module-federation/devtools
@module-federation/cli
create-module-federation
@module-federation/dts-plugin
@module-federation/enhanced
@module-federation/error-codes
@module-federation/esbuild
@module-federation/managers
@module-federation/manifest
@module-federation/metro
@module-federation/metro-plugin-rnc-cli
@module-federation/metro-plugin-rnef
@module-federation/metro-plugin-rock
@module-federation/modern-js
@module-federation/modern-js-v3
@module-federation/native-federation-tests
@module-federation/native-federation-typescript
@module-federation/nextjs-mf
@module-federation/node
@module-federation/observability-plugin
@module-federation/playground
@module-federation/retry-plugin
@module-federation/rsbuild-plugin
@module-federation/rspack
@module-federation/rspress-plugin
@module-federation/rstest
@module-federation/runtime
@module-federation/runtime-core
@module-federation/runtime-tools
@module-federation/sdk
@module-federation/storybook-addon
@module-federation/third-party-dts-extractor
@module-federation/treeshake-frontend
@module-federation/treeshake-server
@module-federation/typescript
@module-federation/utilities
@module-federation/webpack-bundler-runtime
@module-federation/bridge-react
@module-federation/bridge-react-webpack-plugin
@module-federation/bridge-shared
@module-federation/bridge-vue3
@module-federation/inject-external-runtime-core-plugin
commit: |
Bundle Size Report11 package(s) changed, 32 unchanged. Package dist + ESM entry
Bundle targets
Consumer scenarios
Total dist (raw): 36.18 MB (+20.9 kB (+0.1%)) Bundle sizes are generated with rslib (Rspack). Package-root metrics preserve the historical report. Tracked subpath exports such as |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 172184f711
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| remoteInsIndex = instances.findIndex( | ||
| (ins) => ins.name === entryGlobalName, | ||
| ); |
There was a problem hiding this comment.
When an aliased remote uses a custom library.name, entryGlobalName is that library global rather than the Module Federation build name used by the runtime instance; this lookup therefore misses the intended instance and can splice an unrelated instance whose name happens to equal the library global. It also ignores buildVersion, so multiple stale builds with the same name can cause the first, wrong version to be removed. Match the instance using the remote's actual build name and versioned ID instead of treating entryGlobalName as an instance identity.
Useful? React with 👍 / 👎.
| if (remoteInsIndex !== -1) { | ||
| const remoteIns = | ||
| CurrentGlobal.__FEDERATION__.__INSTANCES__[remoteInsIndex]; | ||
| const remoteIns = instances[remoteInsIndex]; | ||
| remoteInsId = remoteIns.options.id || remoteInsId; |
There was a problem hiding this comment.
When this fallback successfully finds an aliased remote, its shared entries are registered with from and useIn values based on the remote instance's build name, but the cleanup below still compares and filters using the registered alias in remoteInfo.name. Consequently none of those entries are queued for deletion before the instance is spliced, leaving stale factories in the host share scope that can be selected after the replacement remote loads; derive the cleanup name from the matched remoteIns.
Useful? React with 👍 / 👎.
|
Context from #4566:
|
…icitly during removal Introduce resolveRemoteRuntimeInstance with monotonic matching (registered name + buildVersion, entryGlobalName + buildVersion, then name-only levels only when no buildVersion is known), split removeRemote into single-purpose steps, release shares by the instance's own producer name, and warn only on ambiguous or version-mismatched lookups. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
ScriptedAlchemy
changed the title
1 participant
Summary
Found while investigating #4566.
RemoteHandler.removeRemote(called byregisterRemotes(remotes, { force: true })when a name is re-registered) looks up the remote's own runtime instance in__FEDERATION__.__INSTANCES__so it can release the remote's share-scope entries and splice the instance out. It matched only on the registeredname(orname:buildVersion), so whenever the registration alias differed from the name the remote was built with, the instance leaked on every forced re-registration, silently.This PR makes the identity used for that lookup explicit and monotonic, releases shares by the instance's own name, and only warns when the situation is actually suspicious.
Identity model
A loaded remote is known under five distinct names, and the code now keeps them apart:
remoteInfo.namehost.options.remotes,moduleCache, snapshotsremoteInfo.entryGlobalNameinstance.options.id__SHARE__map keyinstance.options.name ?? instance.nameshared.from/shared.useInhost.options.nameMatching order (
resolveRemoteRuntimeInstance)New pure helper
packages/runtime-core/src/remote/resolveRemoteRuntimeInstance.ts. The first level that yields any candidate wins; a level with more than one candidate returnsambiguous: trueand no instance (nothing is guessed):buildVersion—options.id === name:buildVersion, oroptions.name === name && options.version === buildVersionentryGlobalName+buildVersion— same two comparisons, only whenentryGlobalNamediffers from the registered namebuildVersionis knownentryGlobalNamealone — only when nobuildVersionis known and exactly one instance matchesA versioned lookup never degrades into an unversioned name match.
Share release
removeRemoteis split into single-purpose private steps run in the previous order under the existingtry/catch:removeRemoteRegistration,clearRemoteSnapshots,clearRemoteEntryState(container global,globalLoading,moduleCache), thenreleaseRuntimeInstanceShares. Share release derivesproducerName = instance.options.name ?? instance.nameand uses it forshared.from;useInonly dropsproducerName(the remote consuming its own share), never the registration alias.isAllSharedNotUsed/needDeleteKeyssemantics and theglobalShareScopeMap[instance.options.id || producerName]deletion are unchanged.Warning
logger.warnfires only when:entryGlobalNamebut none with the requested build version (message lists the versions found).A container with no runtime instance at all (plain containers) is the legitimate case and stays silent.
Tests
packages/runtime-core/__tests__/register-remotes.spec.ts, mirrored inpackages/runtime/__tests__/register-remotes.spec.ts(synthetic instances injected into__INSTANCES__):entryGlobalName= instance name): resolved and removed, no warninguseInno longer contains the remote's own nameoptions.idis custom but whoseoptions.name/options.versionmatch: still resolvedpackages/runtime-core/__tests__/resolve-remote-runtime-instance.spec.tsunit-tests every level, the versioned-never-falls-back rule, ambiguity, andentryGlobalName === namenot forming a level.pnpm --filter @module-federation/runtime-core test(142 passed) andpnpm --filter @module-federation/runtime test(101 passed).A
patchchangeset for@module-federation/runtime-coreis included.🤖 Generated with Claude Code