Improve detection of "root" project when inside `node_modules`

[charlespierce]

When running a shim, Volta will walk up the directory tree to find the closest available package.json file to use as the project, with one exception: When the immediate parent of the directory that contains package.json is node_modules, then we ignore that file. This means when we are inside of our dependencies, if we run a command, we still correctly detect the actual Project root (as opposed to the root of the dependency we are looking at).

Unfortunately, when we are looking at scoped packages (e.g. @babel/core), then the immediate parent is the scope directory, so we think that the root of our project is /project/node_modules/@babel/core, instead of /project. We should also handle the case where the parent is a scoped directory and look one level higher for node_modules.

This may take some design work to make sure that we don't add too much speculative I/O to the project detection process, because it is on the hot path for all commands.

[liamjones]

Just hit a real-world problem with this that's slightly different to the scoped package scenario that was described above so I thought it was worth listing.

I'm using volta with a React Native project. Mostly works fine, I've submitted a PR to the project to add volta support to RN's node detection. However, as part of the iOS builds there's a custom Cocoapods script run to do some code-gen. This does find volta's node shim with my PR above but the problem is, the script being run essential calls:

~/.volta/bin/node ~/[project_directory]/node_modules/react-native-codegen/lib/cli/combine/combine-js-to-schema-cli.js

So volta complains it doesn't know what version of node to use. I can probably patch-package the package.json on install to add a volta extends directive.

A suggestion for how this might work; if the first found package.json doesn't contain a volta section, continue up the tree examining package.json's until you find one that has volta configuration (or tree is exhausted)?

[liamjones]

Actually, scratch that last comment, something else seems to be happening here, I'll raise an issue...

[chriskrycho]

@charlespierce thinking this through: when we walk the path, doe we have to do the actual I/O explicitly at every step up the tree, because of symlinks? Or can we do something simpler using a "dumb" check on the path segments for a given path? I think we could improve the state of affairs at least a little by doing the latter, even if we ultimately determine that it is not sufficient to fully solve the problem?

[charlespierce]

Yeah, I think we could do a naive check on the path segments. We effectively do that already for the node_modules check, at a quick glance this feels like it would be a similar type of check:

• Parent is node_modules (the current check) or
• Parent starts with @ and grandparent is node_modules

Both should be doable on the PathBuf object without needing to do more direct I/O, unless I’m mistaken (been a while since I wrote that so not sure if there was a specific case I was thinking of originally).

On Tue, Dec 19, 2023, at 5:45 PM, Chris Krycho wrote:

@charlespierce https://github.com/charlespierce thinking this through: when we walk the path, doe we have to do the actual I/O explicitly at every step up the tree, because of symlinks? Or can we do something simpler using a "dumb" check on the path segments for a given path? I think we could improve the state of affairs at least a little by doing the latter, even if we ultimately determine that it is not sufficient to fully solve the problem?

— Reply to this email directly, view it on GitHub https://github.com/volta-cli/volta/issues/950#issuecomment-1863712672, or unsubscribe https://github.com/notifications/unsubscribe-auth/AENNW7S3FI6Q4NBNIMTF3WTYKI7K3AVCNFSM4YHL2KO2U5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TCOBWGM3TCMRWG4ZA. You are receiving this because you were mentioned.Message ID: @.***>

[chriskrycho]

Excellent! I have tagged this as a good first issue for folks, then. I believe this is the relevant code:

https://github.com/volta-cli/volta/blob/6a2bc4d1848e3effcbd44b075f5863396fdaf16e/crates/volta-core/src/project/mod.rs#L250-L275

The fix should be fairly straightforward!