Skip to content
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.

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

🪲 Bug: Doesn't work with Webpack Module Federation #140

Closed
tgelu opened this issue Jul 4, 2023 · 10 comments
Closed

🪲 Bug: Doesn't work with Webpack Module Federation #140

tgelu opened this issue Jul 4, 2023 · 10 comments
Assignees
Labels

Comments

@tgelu
Copy link

tgelu commented Jul 4, 2023

The two package.json files from the build output esm/package.json and cjs/package.json which each contain one line confuse webpack module federation and consequently do not load properly in a federated micro frontend app.

Code example
I cannot show a code example but I can explain how the error manifests itself.
Webpack's Module Federation does many things, but at some point it tries to understand the installed version of each dependency. When it reaches react-animate-height it looks for the package.json and reads the version from there. But because react-animate-height has more nested package.json files (which isn't a standard) it finds the deepest one which only contain the type property and isn't a valid package.json file. This confuses the build system because it sees a dependency which is installed but with no version or name.

The error from webpack looks like this:

WARNING in shared module react-animate-height -> /node_modules/react-animate-height/dist/esm/index.js
No version specified and unable to automatically determine one. No version in description file (usually package.json). Add version to description file, or manually specify version in shared config.

Expected behavior
react-animate-height should work in a federated app. The build output should not contain any other package.json files than the root one. (as an aside: I am pretty sure the import and require entries in the root package.json are enough for build systems to know where to locate the entry points, this is pretty standard. I know of no reason to add those extra package.json files)

Possible Solution
In the root package.json:

 - "build:esm": "tsc -p tsconfig.json && echo '{ \"type\": \"module\" }' > dist/esm/package.json",
 - "build:cjs": "tsc -p tsconfig-cjs.json && echo '{ \"type\": \"commonjs\" }' > dist/cjs/package.json",
 + "build:esm": "tsc -p tsconfig.json",
 + "build:cjs": "tsc -p tsconfig-cjs.json",

Steps to Reproduce (for bugs)

  1. Checkout any example of two apps loading each other in a micro frontend with webpack Module Federation (like this one)
  2. Install reeact-animate-height in both apps, the host and the remote.
  3. Try to run the apps and see them fail.

Screenshots
N/A

Your Environment
N/A

Additional context
We're building a micro frontend application with webpack and can't use this package.

@Stanko
Copy link
Owner

Stanko commented Jul 4, 2023

Hi @tgelu,
Thank you for the detailed description. I haven't used module federation so far, but I'll try to setup a small app and look into it this week.

The reason for two package.json files is that bundlers do get confused in some cases (this post describes issues I encountered).

But if I read it correctly, the error is about missing version. Do you think that adding a version to generated package.json files would be enough?

Cheers!

@Stanko Stanko self-assigned this Jul 4, 2023
@Stanko Stanko added the bug label Jul 4, 2023
@tgelu
Copy link
Author

tgelu commented Jul 4, 2023

I am not familiar with the issues described in detail in that article. I have not had any major issue with using the exports field with import and require. Something like the below:

    "exports": {
        ".": {
            "import": "./dist/mjs/index.js",
            "require": "./dist/cjs/index.js"
        }
    }

But I may lack some knowledge of edge cases or other things, so I'm not going to comment on that.

Do you think that adding a version to generated package.json files would be enough?

From my own tests, seems like it would be enough, yes.

PS: it still seems a little bit like those extra package.json files try to "trick" some build systems in some ways, so IMHO it is still not the prudent choice :D. But this is besides the point.

@Stanko
Copy link
Owner

Stanko commented Jul 5, 2023

For now, I patched it in version 3.1.2 by adding version to both package.json files:

But I will look into solving it properly.

EDIT: I would be grateful if you can check if this works around the problem.

@tgelu
Copy link
Author

tgelu commented Jul 10, 2023

Seems to be working now :) - thanks a lot!

Feel free to close the issue.

@tgelu
Copy link
Author

tgelu commented Jul 10, 2023

UPDATE:

I did not test properly. The version warning is gone but now I get a new warning:

[webpack-dev-server] WARNING in shared module react
No required version specified and unable to automatically determine one. Unable to find required version for "react" in description file (node_modules/react-animate-height/dist/esm/package.json). It need to be in dependencies, devDependencies or peerDependencies.

So I am guessing webpack is still confused about this package's package.json file :(

@Stanko
Copy link
Owner

Stanko commented Jul 10, 2023

Heh, It was too easy to be true.

I'll try to publish a beta version today with the solution you proposed initially. Would you be so kind to test it, so I can publish the fix afterwards?

@Stanko
Copy link
Owner

Stanko commented Jul 10, 2023

@tgelu I published a beta version which removes additional package json files:
https://cdn.jsdelivr.net/npm/[email protected]/package.json

From what I can see, it works with both CJS and ESM modules.

Please test it yourself and I'll publish a new version

npm i react-animate-height@beta

@tgelu
Copy link
Author

tgelu commented Jul 10, 2023

Getting no warnings on the @beta release 👍

@Stanko
Copy link
Owner

Stanko commented Jul 10, 2023

Thanks for checking.

I've just published v3.2.0, I think that is it.

@tgelu
Copy link
Author

tgelu commented Jul 11, 2023

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants