-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
fix(module-federation): support buildable libs #20786
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
☁️ Nx Cloud ReportCI is running/has finished running commands for commit e31b64a. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution ✅ Successfully ran 6 targets
Sent with 💌 from NxCloud. |
@@ -57,12 +62,32 @@ function getLibraryImportPath( | |||
library: string, | |||
projectGraph: ProjectGraph | |||
): string | undefined { | |||
let buildLibsFromSource = true; | |||
if (process.env.NX_BUILD_LIBS_FROM_SOURCE) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can find this info from NX_TASK_TARGET_PROJECT
and NX_TASK_TARGET_TARGET
? Do we need the extra env var from executor?
Users won't be able to override with CLI options like --buildLibsFromSource=false
but I think that's fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used a separate one because of cases where a new process isn't spawned, those env vars match the parent process (the host) and not the child (the remote) and for cases where the parent process is actually a serve
that needs to do a build first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, interesting. Maybe something to clean-up later if we can move buildLibsFromSource
to a normal webpack plugin then we wouldn't need all the env vars to be passed like this.
This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request. |
Current Behavior
Buildable libs are not consumed from dist when using MF when creating shared deps
Expected Behavior
Buildable libs should be consumed from dist when creating shared deps
Related Issue(s)
Fixes #20491