Hyphen in module federation host project name causes remotes application to crash in dev mode

jesperume commented 1 month ago

Current Behavior

It's not possible to upgrade to NX 20 when a react host in module federation contains a hyphen in the project name.

Expected Behavior

It's should be possible to upgrade to NX 20 with the new Module federation 2.0 runtime continuing to use a hyphen in the project name for a host, for example "app-shell".

GitHub Repo

No response

Steps to Reproduce

Create an empty Nx project
Add a react host and remote to the project. Notice the hyphen in the host name: npx nx generate @nx/react:host --directory=apps/app-shell --bundler=webpack --name=app-shell --remotes=remote1 --dynamic=true --e2eTestRunner=none --no-interactive
Add the following code to /apps/remote1/src/app/app.tsx so it will contain shared state (to verify that singleton works and react-router-dom is shared between host and remote)
```
<Routes>
<Route path="/" element={<div>Content</div>} />
</Routes>
```

Running npx nx run app-shell:serve will work when the remote will be built and served statically

Running npx nx run app-shell:serve --devRemotes=remote1 will not work as the react application will crash due to "Cannot read properties of null (reading 'useContext')"

After hours of troubleshooting I can see that the host name in http://localhost:4200/remoteEntry.js is incorrect and makes the the remote to not reuse the hosts resources. The failing code is below. Notice the app-shell name has not been transformed as it have in other places where "app-shell" has been transformed to "app_shell" (with underscore).

// process.env.NX_MF_DEV_REMOTES is replaced by an array value via DefinePlugin, even though the original value is a stringified array.
runtimeStore.devRemotes = ["remote1","app-shell"];

This patch will make it work and I hope it helps figuring out where the changes need to be made. Notice that in the repo there is several places where process.env.NX_MF_DEV_REMOTES will be set.

diff --git a/src/executors/module-federation-dev-server/module-federation-dev-server.impl.js b/src/executors/module-federation-dev-server/module-federation-dev-server.impl.js
index 8096646e1f68e11b8b88d274de7053e0f8823a07..1b40e9a3bee0057ebaafd3dfea851a5671120683 100644
--- a/src/executors/module-federation-dev-server/module-federation-dev-server.impl.js
+++ b/src/executors/module-federation-dev-server/module-federation-dev-server.impl.js
@@ -131,7 +131,7 @@ async function* moduleFederationDevServer(options, context) {
     // Set NX_MF_DEV_REMOTES for the Nx Runtime Library Control Plugin
     process.env.NX_MF_DEV_REMOTES = JSON.stringify([
         ...(remotes.devRemotes.map((r) => typeof r === 'string' ? r : r.remoteName) ?? []),
-        p.name,
+        p.name.replace("-", "_"), // Workaround to fix host name issue
     ]);
     const staticRemotesConfig = (0, parse_static_remotes_config_1.parseStaticRemotesConfig)([...remotes.staticRemotes, ...remotes.dynamicRemotes], context);
     const mappedLocationsOfStaticRemotes = await (0, build_static_remotes_1.buildStaticRemotes)(staticRemotesConfig, nxBin, context, options);

Nx Report

Node : 20.17.0 OS : win32-x64 Native Target : x86_64-windows npm : 10.5.0

nx : 20.0.1 @nx/js : 20.0.1 @nx/jest : 20.0.1 @nx/eslint : 20.0.1 @nx/workspace : 20.0.1 @nx/cypress : 20.0.1 @nx/devkit : 20.0.1 @nx/eslint-plugin : 20.0.1 @nx/playwright : 20.0.1 @nx/react : 20.0.1 @nx/vite : 20.0.1 @nx/web : 20.0.1 @nx/webpack : 20.0.1 typescript : 5.5.4

Registered Plugins: @nx/webpack/plugin @nx/eslint/plugin @nx/playwright/plugin @nx/jest/plugin

Failure Logs

No response

Package Manager Version

No response

Operating System

[ ] macOS
[ ] Linux
[ ] Windows
[ ] Other (Please specify)

Additional Information

Please make it possible to continue to use hyphens in host project names. I hope you can revisit the other change that has been made in remotes that can no longer contain hyphens. In javascript developlement kebab-case are golden standard for directories and therefore it won't make sense to force project names to not be able to contain hyphens.

barrymichaeldoyle commented 1 month ago

We have the same problem preventing us from upgrading to v20. There was a PR that partially fixed this in v20.0.1 but it didn't catch everything.

Coly010 commented 1 month ago

@jesperume Good catch! And apologies for the disruption around this, and apologies to @barrymichaeldoyle too.

There were two different changes in Nx 20 which caused these issues as a side effect. From my testing, I didn't encounter the dev remote issue, but I understand it.

I'll get this fixed asap.

barrymichaeldoyle commented 1 month ago

@Coly010 you absolute legend! Thank you 🎉

And thank you @jesperume for logging the issue 👍

github-actions[bot] commented 1 week ago

This issue has been closed for more than 30 days. If this issue is still occuring, please open a new issue with more recent context.

nrwl / nx