volta-cli / rfcs

RFCs for changes to Volta
Other
17 stars 14 forks source link

Better support for monorepos by continuing to search if first encountered package.json does not declare "volta" #51

Open cspotcode opened 2 years ago

cspotcode commented 2 years ago

Alternative to #50 Discussed on Discord here: https://discord.com/channels/413753499835301908/413753499835301917/982039620701270046

Proposed Behavior

When volta needs to discover package.json configuration to choose a node version, it should:

a) ascend the filesystem to the root, stopping at the first package.json with a volta declaration b) if a package.json fails to parse, skip it. Do not abort with error. c) if package.json does not contain "volta", skip it. Do not stop; continue to ascend

Use-case

I have a bunch of automated tests that are all mini-projects, programmatically created, run, and deleted. I was caught off-guard when all those mini-projects were using a different node version, since their package.json files -- little more than {"type": "module"} -- do not declare a volta config, thus my project's pinned node version is ignored.

Performance

In common usage patterns, you will almost always be parsing either a single package.json or none. This proposed change might add stat calls, but not JSON parsing

When running outside of any project

Performance is identical to today

When running inside a single-package project

This change adds additional stat calls, but no additional JSON parsing.

In monorepo cases

Same as for single-package projects, will do exact same amount of JSON parsing: 2x package.json files May do additional stat calls if the monorepo package.json does not declare "volta"

To avoid additional stat calls

I'm guessing that users will be fine with the additional stat calls. However, if someone really wants to avoid them, they can add "volta" declaration to a package.json. Volta will parse it and will not ascend further.

This change allows volta to ascent past package.json files that do not declare "volta", but volta will still stop at package.json that declares "volta". "extends" can be used to explicitly refer to a parent package.json.

Gracefully handling invalid JSON

This change suggests that invalid JSON should be skipped, not treated as a fatal error. I believe this is appropriate because:

a) node-based tools are often a part of development workflows b) Temporarily-invalid package.json is a normal part of development workflows. For example, when resolving merge conflicts, package.json will fail to parse.

Whenever package.json is temporarily un-parse-able, we should not be holding node-based tools hostage. If we do this, we make node-based tools second-class citizens. For example, we cannot implement a git-merge-conflict-resolver CLI tool and install it globally, because volta will occasionally prevent us from using it.

I can extract this proposal to a separate RFC if appropriate.

Debugging

When volta skips invalid JSON, users may want a trace of all package.json files volta is parsing, or failing to parse. VOLTA_LOG_LEVEL or a new env var can do this. For simple cases, volta list is probably best.