Repository navigation
fix(config-nx-scopes): read scopes from the nx project graph - #4957
escapedcat merged 4 commits into
Conversation
|
Thanks! Created this response in cooperation with Claude after going through different points. Let me know if this makes sense for you.
|
…graph Read the nx project graph with spawnSync instead of fork, so getProjects keeps returning string[] and the fix stays a patch. The reader writes its JSON payload to file descriptor 3, a channel nx, its daemon and the worker processes it runs plugins in never write to, so their output on stdout cannot corrupt it.
|
Done, and it stays a patch. I did take a different channel than stdout, and the reason is worth a paragraph because it also answers the wrinkle you flagged. Muzzling nx's output inside the reader does not work. Nx runs workspace plugins in worker processes that inherit the reader's stdout, which I confirmed by printing pid and ppid from inside a fixture plugin and getting a different pid back. Replacing So the payload goes to file descriptor 3. The parent spawns with There is a regression test for it. I also set 1216 tests pass with typecheck clean. Four of the five new assertions fail against master, and the fifth is the synchronous return, which is there to catch a future regression rather than to fail today. One ask: the workflow runs are sitting at |
|
Thanks! All good, just one last small thing to improve:
|
…raph reader The descriptor the reader writes its payload to was declared in index.js and again in project-graph.js, and the two had to agree by hand. The parent now owns the number and passes it to the reader on its command line, so it is declared once and the two sides cannot drift apart.
|
Done in 5d1550d. I went with the argv option, so the parent owns the number and hands it to the reader. The reason I did not use a shared module: it would mean a third file in the package, and The child now takes the descriptor from I checked the drift case rather than assuming it. Changing only the parent to fd 4, with the stdio array widened to match, still passes 18 of 18, because the child follows what it is given. Under the previous code that same one-sided edit produced an empty payload with no error. Full suite is 1215 passing with typecheck clean. The single failure is the One thing outside my control: the workflow runs are still sitting at |
013c358

Description
@commitlint/config-nx-scopesbuilt its scope list by callinggetProjects()fromnx/src/generators/utils/project-configuration.jsagainst aFsTree. That helper is agenerator utility and only reports projects that have a
project.jsonor apackage.jsonon disk. A project that an nx plugin contributes through
createNodeshas neither, so itnever reached
scope-enumand committing against it failed with "scope must be one of[...]".
The config now reads the nx project graph instead.
createProjectGraphAsync()runs theworkspace's plugins before returning, so plugin-inferred projects show up alongside
everything that was already being found. Project names come from
graph.nodes, and eachnode's
datastill carries theprojectTypeandtagsthat thegetProjectsselectoris documented to receive.
One detail worth calling out. Nx works out its workspace root from the cwd of the process
that loads it, then caches that value in places that are not re-read later, including its
daemon client and its cache directory paths. Reassigning the root afterwards gives you a
graph for the right directory but a cache and a daemon keyed to the wrong one. So the
graph is read in a child process started in
ctx.cwd, andspawnSyncblocks until thatprocess exits. That keeps
ctx.cwdbehaving as it does today and keeps nx free to answerfrom its daemon when the user has one. The new
project-graph.jsis the child entry pointand is added to the package's
files.Keeping nx's output out of the payload
The reader hands its JSON back on file descriptor 3 rather than stdout, because stdout is
not ours to keep clean. Nx prints progress there, and it runs workspace plugins in worker
processes that inherit the reader's stdout, so anything a plugin logs arrives interleaved
with the payload. Replacing
process.stdout.writeinside the reader does not help, sincethose writes come from a different process. A sentinel line the parent slices on would
survive the ordinary case, but it still shares one channel with output we do not control,
from processes that can print after the payload has started.
A child's
inheritstdio only covers the first three descriptors, so a fourth pipe is achannel nothing below the reader can write to. The parent asks for
stdio: ["ignore", "pipe", "pipe", "pipe"]and readsresult.output[3]; stdout andstderr are captured for the error message and nothing else.
maxBufferisInfinitybecause the default 1 MB cap counts the payload and nx's chatter together, and it aborts
the read with
ENOBUFSrather than truncating, which a large workspace or a chatty plugincan reach.
The descriptor is declared in exactly one place. The parent owns it and passes it to the
reader as its first argument, and the reader writes to whatever it is handed, rejecting a
missing or non-numeric one. Changing the number on the parent side is enough; there is no
second declaration to keep in step, so the two cannot drift into an empty payload.
fixtures/noisypins the behavior. Its plugin writes to stdout twice while inferring aproject, once through
console.logand once straight to fd 1, and one of those lines is aJSON array itself, so a reader that hunts for JSON in stdout would pick the wrong one.
With the payload on fd 3 the test passes. Moving the payload to stdout and rerunning that
fixture fails with
SyntaxError: Unexpected token 'o', "noisy-plugi"... is not valid JSON, which is how I checked the fixture reproduces the problem instead of onlydescribing it.
The exported
utils.getProjectsutils.getProjectskeeps its signature and itsstring[]return value. The graph isbuilt asynchronously inside the child, but
spawnSyncwaits for the result, so nothingabout the exported signature moves and
scope-enumgoes back to its plain form.Existing configs need no edit and this ships as a patch.
An earlier revision of this PR made
getProjectsasync and I flagged the return type foryour call. @escapedcat asked for the synchronous version and that is what this branch now
does, so the question is settled and the breaking-change checkbox stays unticked. For a
hook that runs once per commit there is nothing else in flight while nx builds the graph,
so async bought us nothing anyway.
There is a second, smaller behavior change that comes with reading the graph. Nx's own
package-json plugin attaches an
npm:publictag to projects that have a publicpackage.json, so selectors that inspecttagswill now see tags that nx adds ratherthan only the ones written in
project.json. Existing selectors keep working because thechange is additive.
nx peer range
Unchanged at
nx: >=14.0.0.createProjectGraphAsyncis exported fromnx/src/project-graph/project-graph.jsacross that whole range. I checked the publishedtarballs for 14.0.0, 14.1 through 14.5, 14.8.6, 15.0.0, 15.9.7, 16.0.0, 16.10.0, 17.0.0,
17.3.2, 18.0.0, 19.0.0, 20.0.0, 21.0.0 and 22.0.0, and 23.1.1 as installed in this repo,
where the file now resolves through nx's
exportsmap todist/. No fallback path and nopeer bump are needed.
Motivation and Context
Fixes #4861
The scope list is meant to be "the projects in this workspace". Any workspace using an nx
plugin that infers projects, which is the default shape of a modern nx workspace, was
getting a partial list and had no way to commit against the missing projects.
Usage examples
Given a workspace where
nx show projectslistsapi,webanddocs, anddocsonlyexists because a plugin inferred it:
How Has This Been Tested?
Two new fixtures.
fixtures/inferredholds a small nx plugin intools/inferring-plugin.jsthat turns anx/*/inferred.markerfile into a project. The fixture hasfixture-inferred-i, whichexists only through that plugin and has no
project.jsonorpackage.json, next tofixture-inferred-j, which is an ordinaryproject.jsonproject. Having both means thetests show the change adding projects rather than swapping one discovery mechanism for
another.
fixtures/noisyholds a plugin that prints to stdout while it infersfixture-noisy-k,covering the stdout question above.
Tests added to
index.test.js:scope-enumon the inferred fixture returns both the inferred and the explicit projectscope-enumon the noisy fixture returns the project and ignores what the plugin printedutils.getProjectsis still exported and still returns the project names on the existingbasicfixture, without a promise, which pins the back compat decision abovetags, filtering down tofixture-inferred-iprojectTypeFour of those five fail against
masterwith assertion errors rather than import or builderrors. The fifth is the synchronous return value, which passes on
masteron purpose:it exists to catch a regression in what this PR must not change. The tests set
NX_DAEMON=false, because each fixture runs in its own throwaway directory and a daemonstarted for one of them would outlive the run with nothing left to watch.
What I ran locally on Node 22.12 and pnpm 11.22:
pnpm buildpnpm test(the full suite, 91 files and 1216 tests, plus typecheck, all passing)pnpm lintandpnpm format, both cleanI also confirmed by hand that the daemon path works, by running the fixtures once without
NX_DAEMON=falseand getting the same project lists, and that no daemon processes or.nxdirectories are left behind after
pnpm test. Payloads with multi-byte project names andpayloads several MB large come back over fd 3 intact.
My machine is macOS, so the fourth stdio pipe is the piece I would most want a second
platform on; the CI matrix exercises it on Windows and Ubuntu across Node 22 and 24.
What I did not run:
pnpm run pkgfails in my checkout for@commitlint/config-nx-scopesand equally for untouched packages such as
@commitlint/config-lerna-scopes, because thepacked tarball cannot resolve
workspace:dependencies outside the monorepo. That lookslike a local environment limitation rather than anything this change introduced, so I have
left it to CI. I have not tested against an nx version older than 23 at runtime, since the
repo installs a single nx; the peer range claim above rests on reading the published
packages.
Types of changes
The return type question that was open here is resolved:
utils.getProjectsstill returnsstring[], so nothing on the public surface changes and this is a bug fix.Checklist:
then the implementation.