-
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(misc): ensure plugins are not creating workspace context while creating nodes #22967
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
☁️ Nx Cloud ReportCI is running/has finished running commands for commit 2781256. 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 3 targetsSent with 💌 from NxCloud. |
b3e8682
to
0cbad41
Compare
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.
Playwright is actually also broken.. but would be fixed if you convert all the functions in the WorkspaceContext
to go to the daemon.
None of the calls to the workspace context should stay on the process unless the daemon is disabled or its on the daemon itself. Things like the Nx process should also go to the daemon to not do double work.
packages/nx/src/utils/globs.ts
Outdated
export async function globAsync(globs: string[], exclude?: string[]) { | ||
if (isOnDaemon() || !daemonClient.enabled()) { | ||
return globWithWorkspaceContext(workspaceRoot, globs, exclude); | ||
} else { | ||
return daemonClient.glob(globs, exclude); | ||
} | ||
} |
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 think we can do this in the workspace-context.ts
file.
Those functions should actually all go to the daemon...
Things like allWorkspaceFiles
are also broken right now.
Could we change them all?
0cbad41
to
5316523
Compare
5316523
to
2781256
Compare
Closing in favor of #26253 |
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. |
…eating nodes
Current Behavior
Some plugins utilize
globWithWorkspaceContext
to perform a glob search duringcreateNodes
. This works fine without plugin isolation, and would still "work" with it, but when plugins are isolated they don't share a workspace context. Combine this with the daemon and we have issues.Expected Behavior
If the daemon is enabled, its workspace context is utilized when performing glob searches
Related Issue(s)
Fixes #