Skip to content

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

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

Closed
4 tasks
jesperume opened this issue Oct 17, 2024 · 4 comments · Fixed by #28512
Closed
4 tasks
Assignees
Labels
outdated scope: module federation Issues related to module federation support type: bug

Comments

@jesperume
Copy link

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
Copy link
Contributor

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 Coly010 self-assigned this Oct 18, 2024
@Coly010
Copy link
Contributor

Coly010 commented Oct 18, 2024

@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
Copy link
Contributor

@Coly010 you absolute legend! Thank you 🎉

And thank you @jesperume for logging the issue 👍

jaysoo pushed a commit that referenced this issue Oct 18, 2024
…trol plugin #28497 (#28512)

<!-- Please make sure you have read the submission guidelines before
posting an PR -->
<!--
https://github.com/nrwl/nx/blob/master/CONTRIBUTING.md#-submitting-a-pr
-->

<!-- Please make sure that your commit message follows our format -->
<!-- Example: `fix(nx): must begin with lowercase` -->

<!-- If this is a particularly complex change or feature addition, you
can request a dedicated Nx release for this pull request branch. Mention
someone from the Nx team or the `@nrwl/nx-pipelines-reviewers` and they
will confirm if the PR warrants its own release for testing purposes,
and generate it for you if appropriate. -->

## Current Behavior
<!-- This is the behavior we have today -->
As part of the work to normalize `-` in MF project names for Federation,
setting the NX_MF_DEV_REMOTES env var was missed.
This causes issues with the RuntimeLibraryControlPlugin


## Expected Behavior
<!-- This is the behavior we should expect with the changes in this PR
-->
Remote names should be normalized correctly

## Related Issue(s)
<!-- Please link the issue being fixed so it gets closed when this is
merged. -->

Fixes #28497
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated scope: module federation Issues related to module federation support type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants