Fix shared dependencies loaded multiple times

[wurstbonbon]

Description

There are many open issues regarding shared dependencies (often react) that are loaded multiple times:
#446
#478
#429
...

Here is my understanding what is going on and were the problems lies. Correct me if I’m wrong, I’m just stumbling through the code trying to make sense of it.


The last version that work well with React and libraries that depend on React is 1.1.14.
With 1.2.0 came this commit 49b6cae It moved the transformation of the shared module imports from generateBundle to transform.

This is not working well together with the CommonJS plugin. The transform sees commonjsHelpers.js imports and imports like node_modules/react/index.js?commonjs-proxy. It does not transform those imports and they end up as direct imports instead of importShared() calls. This will then cause two instances of e.g. react to be imported one from the host and one from the remote. 



I made a ham-fisted attempt to fix this by adding a second import conversion round in generateBundle. This sort of reverts the changes done in 49b6cae. Honestly I don’t think this makes a lot of sense, but it could be a starting point for a discussion. Note: The fix does not work for SystemJS. Because by the time generateBundle is called for SystemJS the imports from the CommonJS plugin are no longer recognisable as shared imports.


The commit mentioned above was introduced to fix this issue #334 I’ve tried to reproduce the issue on the given example repo. Using 1.1.14 and even 1.1.12, but was unable to do so. So I don’t understand what the actually initial problem was.
Could be that I'm doing something or that changes in vue cause this to no longer happen.

Maybe a way forward would be to more thoroughly revert back to the initial approach of doing the conversion in generateBundle.




Additional context

---

What is the purpose of this pull request?

• Bug fix
• New Feature
• Documentation update
• Other

Before submitting the PR, please make sure you do the following

• Read the Code of Conduct and follow the Commit Convention guidelines.
• Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
• Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
• Ideally, include relevant tests that fail without this PR but pass with it.

[flyfishzy]

Hi @wurstbonbon, it seems that CI is failed in serve mode.
@ruleeeer , Can you take the time to review the code?

[wurstbonbon]

Hi @flyfishzy, I figured out why the CI was failing. Before my changes the webpack-host example was loading react twice. After my changes the remote tries to use react from the host. This was failing because the react module from webpack is structured differently. Again a common.js issue I'm afraid. I added a workaround by setting commonjsOptions: {defaultIsModuleExports: true} on the remote. I also had to make sure that every shared module exports default so that it can be dynamically imported.

I created a new example / test using react with zustand to demonstrate what has been fixed.

[wurstbonbon]

Hi @flyfishzy, thanks for retriggering the tests. Any chance that failing tests are just a fluke? I've run them locally on macOS with node 16 a couple of times and I can't reproduce the failure. Also strange that it would only fail on macOS with node 16.

[acollazomayer]

Hi @wurstbonbon I just tried this with the exact same scenario as this: #446. And the fix is not working maybe I am missing somethig. I need to add commonjsOptions: {defaultIsModuleExports: true}

Happy to help.

I still have the same error
Uncaught Error: Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for one of the following reasons:

[wurstbonbon]

Hi @acollazomayer, are both host and remote using vite? I had to add commonjsOptions: {defaultIsModuleExports: true} when using a webpack host.

How is what you are doing different from the example that I added?

I tried sharing react-router-dom works fine.

[satyam-veris]

@wurstbonbon any update on the PR?

[Shelrothman]

@wurstbonbon any updates on this PR? It seems to only be failing on one test now; CI / Build&Test: node-16, macos-latest

[PicoI2]

In remotes, if the shared lib is not generated, this import will not work.
I had to replace it with
'./${this.getFileName(
to fix it.
But this PR does resolve a issues I had where react was loaded multiple times, great job.

[wurstbonbon]

I'm not sure if I can follow.
getFileName includes the asset directory so for me the import ends up being.

<HOST>/asset/asset/__federation_fn_import-<HASH>.js

I did change the code to replace the asset directory with a dot to get a relative import. But I don't know it that addresses the problem that you are having. Help is appreciated :)

[wurstbonbon]

Great that there is finally something happing here 👍

@Shelrothman
As I mentioned before I'm not sure if the failing node-16 macOS tests aren't a fluke. I even tried it with node-16 on a Mac and the tests were successful. Can we re-run the tests? If there is actually a node-16 + macOS specific issue then I'm afraid I need help.

[ReCREWt-Fabi]

@wurstbonbon Any updates on this PR? We'd urgently need this fix to be released to get reactflow to work in a remote component

[Shelrothman]

@wurstbonbon Any updates on this PR? We'd urgently need this fix to be released to get reactflow to work in a remote component

unfortunately, there are no updates as far as I know. My team has had to shift to a new build tool. Now we are trying to use rsBuild and @module-federation/enhanced/rspack to do the "micro-frontending". Although it has its own issues too..

[jean-moreira-ihm]

Do you have any news about this fix? Will it be applied, or are there alternatives that can be done until this solution is implemented?

[pganster]

@Shelrothman can you highlight problems you had with rsbuild?
We are now considering to switch too, as this repository seems to have become stale, as the last change was 5 months ago. I also don't believe that this PR will ever be merged considering the activity.

[Shelrothman]

@Shelrothman can you highlight problems you had with rsbuild? We are now considering to switch too, as this repository seems to have become stale, as the last change was 5 months ago. I also don't believe that this PR will ever be merged considering the activity.

Hey @pganster unfortunately, I don't think I can be of much help. rsBuild is fine if all the remote apps use the same build-tool, but ours do not, so it didn't work out.
My team decided to shift away from trying to micro-frontend for the time being. Instead we are focused on building out a shared component library (via npm package) and have all our apps consume the library. To give a "look" of microfrontending without actually doing it. 🙈

We may revisit later on, but way too much developer time has already been spent on it so it wont be for a while most likely.

[pganster]

Just a heads up: We've now successfully switched all our services from vite + this module federation plugin to rsbuild. RsBuild builds upon the module-federation/core and is for now better maintained. For now we didn't encounter any major issue that prevented us from switching.