Skip to content

fix(mf): use unresolved request to match shared module #11478

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+4 −1
Open

fix(mf): use unresolved request to match shared module #11478

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
df5f9d9
fix(mf): use unresolved request to match shared module
elbywan Aug 25, 2025
6decd96
Merge branch 'main' into fix-mf-shared-custom-resolution
elbywan Aug 27, 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
5 changes: 4 additions & 1 deletion crates/rspack_plugin_mf/src/sharing/provide_shared_plugin.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,10 @@ async fn normal_module_factory_module(
{
return Ok(());
}
let request = &create_data.raw_request;
let dependency = data.dependencies[0]
Copy link
Contributor

Choose a reason for hiding this comment

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

I think a better way is use data.request here, should also solve the issue since dependency.request is assigned from data.request

Copy link
Contributor Author

@elbywan elbywan Aug 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

👋 I tried to do that but it seems that data.request contains the mutated resolved path instead of the original request (since this change?). I could not find another practical way of retrieving it except extracting it from dependency[0].request which does contain the pristine specifier.

Copy link
Contributor

@ahabhgk ahabhgk Aug 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I see, have you tried webpack that they use the unresolved request or the changed request (changed by NormalModuleFactory.hooks.afterResolve)? If I remember correctly they will use the changed request. So at this point I'd like to keep the consistent with webpack

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh there is another issue about ConsumeSharedPlugin not using the changed request #11437 (your PR is about ProvideSharedPlugin not using original request)

Copy link
Contributor Author

@elbywan elbywan Aug 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Ah I see this is more complicated than expected.

I see, have you tried webpack that they use the unresolved request or the changed request (changed by NormalModuleFactory.hooks.afterResolve)? If I remember correctly they will use the changed request. So at this point I'd like to keep the consistent with webpack

Oh, I can double check 👍.

What worries me is that if the logic is kept like this, it would make it very complicated to use a custom resolver - like with packages having subpath "exports" fields - because I'm not sure resolving all these paths ahead of time when generating the MF config + using the import field would be straightforward.

Especially with packages that only export deep subpaths, without root exports. ("exports": {"." { … } })

Oh there is another issue about ConsumeSharedPlugin not using the changed request #11437 (your PR is about ProvideSharedPlugin not using original request)

I'm wondering, what would be the expected outcome in this case? The consumer would load the module from the resolved path and assume that it is not provided by a producer?

In addition to this PR changes, I tried modifying the request to use the resolved path &data.request in the factorize hook here and it seems to solve both issues but I'm not fully sure that I get all the implications yet.

I'll try to dig into it and get a better understanding.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious how you use the custom resolver that resolves paths ahead of time. If you resolve paths before compile, you can use the resolved paths to generate a MF plugin (configured with resolved paths instead of the original paths) and apply to the compiler

Copy link
Contributor Author

@elbywan elbywan Aug 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think that this is precisely the problem, I don't think that I can easily resolve all the possible subpaths for shared modules before compilation because I have no knowledge of the imports in the codebase. For simple cases it works, but as soon as I try to share a module exporting only subpaths it breaks (and the trailing slash syntax - module/ - does not work when using an absolute path fallback).

For instance, for these kind of packages…

{ "name": "package", "main": "index.js" }

…I cannot guess what the imports will look like in the provider code…

// any subpath is technically possible
import json from "package/package.json";
import stuff from "package/stuff";
import otherStuff from "package/other/stuff";
import someOtherStuff from "package/some/other/stuff";

…and it becomes very hard to generate the MF config:

new ModuleFederationPlugin({
  name: "host",
  shared: {
    package: {
      version: "1.0.0",
      import: // I can add the resolved path here but what about nested paths??
    },
    "package/": {
      version: "1.0.0",
      import: // won't work
    },
    // Does every possible import paths have to be added as keys here?
  },
}),

Whereas this config used to work just fine with v1.4.6:

new ModuleFederationPlugin({
  name: "host",
  shared: {
    package: {
      version: "1.0.0",
    },
    "package/": {
      version: "1.0.0",
    },
  },
}),

.as_module_dependency()
.expect("should be module dependency");
let request = dependency.request();
{
let match_provides = self.match_provides.read().await;
if let Some(config) = match_provides.get(request) {
Expand Down
Loading