Skip to content
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

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

Open
cspotcode opened this issue Jun 3, 2022 · 0 comments

Comments

@cspotcode
Copy link

cspotcode commented Jun 3, 2022

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

  • volta will stat for package.json up to filesystem root, will not find any

When running inside a single-package project

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

  • volta will stat for package.json up to filesystem root, but will only encounter one package.json. Thus same amount of parsing work

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"

  • today, is because we are forced to add "extends" to the nested package.json. Volta must parse them both
  • with this change, we will not be forced to add "extends"; volta will still parse them both

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant