Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions .changeset/fix-shared-get-guard.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
'@module-federation/runtime-core': patch
'@module-federation/metro-core': patch
---

fix: guard `targetShared.get` before calling in `loadShare`

When the host's share scope contains a dependency that a remote does not
declare, the share scope entry has no `get()` factory. `loadShare` called
`targetShared.get!()` without checking, crashing with
`targetShared.get is not a function`. Added a typeof guard before both
call sites in `SharedHandler.loadShare`, returning `false` (the documented
miss sentinel) so callers can fall back correctly.

Also guard the `factory()` call in `metro-core`'s `remote-module-registry`
to handle the `false` return from `loadShare` without crashing.

Fixes #2497
7 changes: 6 additions & 1 deletion packages/metro-core/src/runtime/remote-module-registry.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ export async function loadSharedToRegistryAsync(id) {
registry[id] = {};
loading[id] = (async () => {
const factory = await loadShare(id);
if (!factory) {
return;
}
const sharedModule = factory();
cloneModule(sharedModule, registry[id]);
})();
Expand All @@ -67,7 +70,9 @@ export function loadSharedToRegistrySync(id) {
return;
}
loading[id] = loadShareSync(id);
registry[id] = loading[id]();
if (loading[id]) {
registry[id] = loading[id]();
}
}

export function getModuleFromRegistry(id) {
Expand Down
10 changes: 8 additions & 2 deletions packages/runtime-core/src/shared/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,10 @@ export class SharedHandler {
return factory;
} else {
const asyncLoadProcess = async () => {
const factory = await targetShared.get!();
if (typeof targetShared.get !== 'function') {
return false as unknown as () => T;
}
Comment on lines +192 to +194
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a valid point, will update

const factory = await targetShared.get();
addUseIn(targetShared, host.options.name);
targetShared.loaded = true;
targetShared.lib = factory;
Expand Down Expand Up @@ -217,7 +220,10 @@ export class SharedHandler {
const targetShared = directShare(shareOptionsRes, _useTreeShaking);

const asyncLoadProcess = async () => {
const factory = await targetShared.get!();
if (typeof targetShared.get !== 'function') {
return false as unknown as () => T;
}
const factory = await targetShared.get();
targetShared.lib = factory;
targetShared.loaded = true;
addUseIn(targetShared, host.options.name);
Expand Down
46 changes: 46 additions & 0 deletions packages/runtime/__tests__/shares.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -948,4 +948,50 @@ describe('load share while shared has multiple versions', () => {
assert(sharedRes, "sharedRes can't be null");
expect(sharedRes.version).toEqual('16.0.0');
});

// Regression test for https://github.com/module-federation/core/issues/2497
// When the host's share scope contains a dep the remote doesn't declare,
// the scope entry has no get() factory. loadShare must not crash.
it('loadShare does not crash when share scope entry has no get factory', async () => {
const host = init({
name: '@federation/host-extra-dep',
remotes: [],
shared: {
react: {
version: '18.0.0',
scope: ['default'],
get: () =>
Promise.resolve(() => ({
default: 'react',
version: '18.0.0',
from: '@federation/host-extra-dep',
})),
shareConfig: {
singleton: true,
requiredVersion: '^18.0.0',
},
},
},
});

// Inject a share scope entry without a get() factory, simulating what
// happens when the host registers a shared dep the remote doesn't declare.
const scope = host.shareScopeMap['default'] || {};
host.shareScopeMap['default'] = scope;
scope['moment'] = {
'2.30.0': {
version: '2.30.0',
from: 'remote-without-moment',
scope: ['default'],
shareConfig: {
singleton: false,
requiredVersion: '^2.30.0',
},
} as any,
};

// Should return false (miss sentinel) instead of crashing
const result = await host.loadShare<any>('moment');
expect(result).toBe(false);
});
});