Skip to content

fix(enhanced): Add fallback parsing for container module exposes #4034

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Sep 19, 2025
+51 −1
Merged

fix(enhanced): Add fallback parsing for container module exposes #4034

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
aef90ed
fix(enhanced): add fallback parsing for container module exposes
vinicius-at-webflow Aug 28, 2025
68ebd01
fix(enhanced): add fallback parsing for container module exposes
vinicius-at-webflow Aug 28, 2025
2955c59
Merge branch 'main' into main
vinicius-at-webflow Sep 1, 2025
ba44e59
Merge branch 'main' into fix-handle-container-module
vinicius-at-webflow Sep 6, 2025
ef84efd
chore: remove changes to package-json-version tests and include dedic…
vinicius-at-webflow Sep 6, 2025
b55085a
test: provide public path
vinicius-at-webflow Sep 6, 2025
94d354c
Merge branch 'main' into fix-handle-container-module
ScriptedAlchemy Sep 14, 2025
754c028
Merge branch 'container-json-parse' into fix-handle-container-module
ScriptedAlchemy Sep 19, 2025
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
6 changes: 6 additions & 0 deletions packages/enhanced/test/configCases/container/expose-edge-cases/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
it('should be able to handle spaces in path to exposes', async () => {
const { default: test1 } = await import('./test 1');
const { default: test2 } = await import('./path with spaces/test-2');
expect(test1()).toBe('test 1');
expect(test2()).toBe('test 2');
});
3 changes: 3 additions & 0 deletions packages/enhanced/test/configCases/container/expose-edge-cases/path with spaces/test-2.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function test() {
return 'test 2';
}
3 changes: 3 additions & 0 deletions packages/enhanced/test/configCases/container/expose-edge-cases/test 1.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function test() {
return 'test 1';
}
20 changes: 20 additions & 0 deletions packages/enhanced/test/configCases/container/expose-edge-cases/webpack.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
const { ModuleFederationPlugin } = require('../../../../dist/src');

module.exports = {
mode: 'development',
devtool: false,
output: {
publicPath: 'http://localhost:3000/',
},
plugins: [
new ModuleFederationPlugin({
name: 'remote',
filename: 'remoteEntry.js',
manifest: true,
exposes: {
'./test-1': './test 1.js',
'./test-2': './path with spaces/test-2.js',
},
}),
],
};
20 changes: 19 additions & 1 deletion packages/manifest/src/ModuleHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,25 @@ class ModuleHandler {
// identifier: container entry (default) [[".",{"import":["./src/routes/page.tsx"],"name":"__federation_expose_default_export"}]]'
const data = identifier.split(' ');

JSON.parse(data[3]).forEach(([prefixedName, file]) => {
let entries: Array<
[exposeKey: string, { import: string[]; name?: string }]
> = [];

try {
// Prefer parsing exposes from stats first
entries = JSON.parse(data[3]);
} catch {
// If that fails, fallback to the original options
const exposes = this._options.exposes;

if (!exposes || typeof exposes !== 'object') {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

#4027 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From my investigations this.options.exposes in ModuleHandler is always parsed into

interface ExposesObject {
    [k: string]: ExposesConfig;
}

interface ExposesConfig {
    import: ExposesItem | ExposesItems;
    name?: string;
}

by ContainerManager.containerPluginExposesOptions. But due to how everything is currently typed, TS thinks this could be the broader "unparsed" Exposes shape.

type Exposes = (ExposesItem | ExposesObject)[] | ExposesObject;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is how it looks like, for both enhanced/ModuleFederationPlugin and rspack/ModuleFederationPlugin

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now, we could maintain this 100% decoupled and idempotently "re-parse" the entries ModuleHandler to ensure the type safety, but I would wait for a review first before increasing the complexity here. cc @ScriptedAlchemy @2heal1

return;
}

entries = Object.entries(exposes);
}

entries.forEach(([prefixedName, file]) => {
Comment on lines +295 to +313
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm opting to maintain the current JSON parsing logic and wrap it in a try catch block. There are probably good reason why it was implemented this way in the first place, but I lack the context, and I rather be as safe/backwards compatible as possible. If it fails the introduced logic just attempts to recover exposes from another source.

@2heal1 @ScriptedAlchemy As you mentioned in the issue, there are probably better ways of solving this, but again, I lack the context to propose a long-term solution. You mentioned a short-term solution, but since the issue was closed, I figured it wouldn't hurt proposing this patch 😅.

Also note I'm open to contribute and implement this in a different way if I can get some high level guidance (Just a couple of quick notes would be really helpful).

// TODO: support multiple import
exposesMap[getFileNameWithOutExt(file.import[0])] = getExposeItem({
exposeKey: prefixedName,
Expand Down