Skip to content

feat: enable --resolve-engines by default. out of experimental phase! #1265

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

Merged
merged 5 commits into from
Oct 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .changeset/warm-hounds-change.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
"fnm": minor
---

enable `--resolve-engines` by default. out of experimental phase.

to disable it, add a `--resolve-engines=false` flag, and make sure to open an issue describing _why_.
It might feel like a breaking change but .nvmrc and .node-version have precedence so it should not.

I am all in favor of better experience and I believe supporting engines.node is a good direction.
2 changes: 1 addition & 1 deletion .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ concurrency:
cancel-in-progress: true

env:
RUST_VERSION: "1.78"
RUST_VERSION: "1.81"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related?


jobs:
fmt:
Expand Down
119 changes: 92 additions & 27 deletions docs/commands.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,17 @@ Options:

[env: FNM_COREPACK_ENABLED]

--resolve-engines
--resolve-engines [<RESOLVE_ENGINES>]
Resolve `engines.node` field in `package.json` whenever a `.node-version` or `.nvmrc` file is not present.
Experimental: This feature is subject to change.
This feature is enabled by default. To disable it, provide `--resolve-engines=false`.

Note: `engines.node` can be any semver range, with the latest satisfying version being resolved.
Note 2: If you disable it, please open an issue on GitHub describing _why_ you disabled it.
In the future, disabling it might be a no-op, so it's worth knowing any reason to
do that.

[env: FNM_RESOLVE_ENGINES]
[possible values: true, false]

-h, --help
Print help (see a summary with '-h')
Expand Down Expand Up @@ -137,12 +142,17 @@ Options:

[env: FNM_COREPACK_ENABLED]

--resolve-engines
--resolve-engines [<RESOLVE_ENGINES>]
Resolve `engines.node` field in `package.json` whenever a `.node-version` or `.nvmrc` file is not present.
Experimental: This feature is subject to change.
This feature is enabled by default. To disable it, provide `--resolve-engines=false`.

Note: `engines.node` can be any semver range, with the latest satisfying version being resolved.
Note 2: If you disable it, please open an issue on GitHub describing _why_ you disabled it.
In the future, disabling it might be a no-op, so it's worth knowing any reason to
do that.

[env: FNM_RESOLVE_ENGINES]
[possible values: true, false]

-h, --help
Print help (see a summary with '-h')
Expand Down Expand Up @@ -194,12 +204,17 @@ Options:

[env: FNM_COREPACK_ENABLED]

--resolve-engines
--resolve-engines [<RESOLVE_ENGINES>]
Resolve `engines.node` field in `package.json` whenever a `.node-version` or `.nvmrc` file is not present.
Experimental: This feature is subject to change.
This feature is enabled by default. To disable it, provide `--resolve-engines=false`.

Note: `engines.node` can be any semver range, with the latest satisfying version being resolved.
Note 2: If you disable it, please open an issue on GitHub describing _why_ you disabled it.
In the future, disabling it might be a no-op, so it's worth knowing any reason to
do that.

[env: FNM_RESOLVE_ENGINES]
[possible values: true, false]

-h, --help
Print help (see a summary with '-h')
Expand Down Expand Up @@ -267,12 +282,17 @@ Options:

[env: FNM_COREPACK_ENABLED]

--resolve-engines
--resolve-engines [<RESOLVE_ENGINES>]
Resolve `engines.node` field in `package.json` whenever a `.node-version` or `.nvmrc` file is not present.
Experimental: This feature is subject to change.
This feature is enabled by default. To disable it, provide `--resolve-engines=false`.

Note: `engines.node` can be any semver range, with the latest satisfying version being resolved.
Note 2: If you disable it, please open an issue on GitHub describing _why_ you disabled it.
In the future, disabling it might be a no-op, so it's worth knowing any reason to
do that.

[env: FNM_RESOLVE_ENGINES]
[possible values: true, false]

-h, --help
Print help (see a summary with '-h')
Expand Down Expand Up @@ -334,12 +354,17 @@ Options:

[env: FNM_COREPACK_ENABLED]

--resolve-engines
--resolve-engines [<RESOLVE_ENGINES>]
Resolve `engines.node` field in `package.json` whenever a `.node-version` or `.nvmrc` file is not present.
Experimental: This feature is subject to change.
This feature is enabled by default. To disable it, provide `--resolve-engines=false`.

Note: `engines.node` can be any semver range, with the latest satisfying version being resolved.
Note 2: If you disable it, please open an issue on GitHub describing _why_ you disabled it.
In the future, disabling it might be a no-op, so it's worth knowing any reason to
do that.

[env: FNM_RESOLVE_ENGINES]
[possible values: true, false]

-h, --help
Print help (see a summary with '-h')
Expand Down Expand Up @@ -406,12 +431,17 @@ Options:

[env: FNM_COREPACK_ENABLED]

--resolve-engines
--resolve-engines [<RESOLVE_ENGINES>]
Resolve `engines.node` field in `package.json` whenever a `.node-version` or `.nvmrc` file is not present.
Experimental: This feature is subject to change.
This feature is enabled by default. To disable it, provide `--resolve-engines=false`.

Note: `engines.node` can be any semver range, with the latest satisfying version being resolved.
Note 2: If you disable it, please open an issue on GitHub describing _why_ you disabled it.
In the future, disabling it might be a no-op, so it's worth knowing any reason to
do that.

[env: FNM_RESOLVE_ENGINES]
[possible values: true, false]

-h, --help
Print help (see a summary with '-h')
Expand Down Expand Up @@ -468,12 +498,17 @@ Options:

[env: FNM_COREPACK_ENABLED]

--resolve-engines
--resolve-engines [<RESOLVE_ENGINES>]
Resolve `engines.node` field in `package.json` whenever a `.node-version` or `.nvmrc` file is not present.
Experimental: This feature is subject to change.
This feature is enabled by default. To disable it, provide `--resolve-engines=false`.

Note: `engines.node` can be any semver range, with the latest satisfying version being resolved.
Note 2: If you disable it, please open an issue on GitHub describing _why_ you disabled it.
In the future, disabling it might be a no-op, so it's worth knowing any reason to
do that.

[env: FNM_RESOLVE_ENGINES]
[possible values: true, false]

-h, --help
Print help (see a summary with '-h')
Expand Down Expand Up @@ -532,12 +567,17 @@ Options:

[env: FNM_COREPACK_ENABLED]

--resolve-engines
--resolve-engines [<RESOLVE_ENGINES>]
Resolve `engines.node` field in `package.json` whenever a `.node-version` or `.nvmrc` file is not present.
Experimental: This feature is subject to change.
This feature is enabled by default. To disable it, provide `--resolve-engines=false`.

Note: `engines.node` can be any semver range, with the latest satisfying version being resolved.
Note 2: If you disable it, please open an issue on GitHub describing _why_ you disabled it.
In the future, disabling it might be a no-op, so it's worth knowing any reason to
do that.

[env: FNM_RESOLVE_ENGINES]
[possible values: true, false]

-h, --help
Print help (see a summary with '-h')
Expand Down Expand Up @@ -593,12 +633,17 @@ Options:

[env: FNM_COREPACK_ENABLED]

--resolve-engines
--resolve-engines [<RESOLVE_ENGINES>]
Resolve `engines.node` field in `package.json` whenever a `.node-version` or `.nvmrc` file is not present.
Experimental: This feature is subject to change.
This feature is enabled by default. To disable it, provide `--resolve-engines=false`.

Note: `engines.node` can be any semver range, with the latest satisfying version being resolved.
Note 2: If you disable it, please open an issue on GitHub describing _why_ you disabled it.
In the future, disabling it might be a no-op, so it's worth knowing any reason to
do that.

[env: FNM_RESOLVE_ENGINES]
[possible values: true, false]

-h, --help
Print help (see a summary with '-h')
Expand Down Expand Up @@ -656,12 +701,17 @@ Options:

[env: FNM_COREPACK_ENABLED]

--resolve-engines
--resolve-engines [<RESOLVE_ENGINES>]
Resolve `engines.node` field in `package.json` whenever a `.node-version` or `.nvmrc` file is not present.
Experimental: This feature is subject to change.
This feature is enabled by default. To disable it, provide `--resolve-engines=false`.

Note: `engines.node` can be any semver range, with the latest satisfying version being resolved.
Note 2: If you disable it, please open an issue on GitHub describing _why_ you disabled it.
In the future, disabling it might be a no-op, so it's worth knowing any reason to
do that.

[env: FNM_RESOLVE_ENGINES]
[possible values: true, false]

-h, --help
Print help (see a summary with '-h')
Expand Down Expand Up @@ -713,12 +763,17 @@ Options:

[env: FNM_COREPACK_ENABLED]

--resolve-engines
--resolve-engines [<RESOLVE_ENGINES>]
Resolve `engines.node` field in `package.json` whenever a `.node-version` or `.nvmrc` file is not present.
Experimental: This feature is subject to change.
This feature is enabled by default. To disable it, provide `--resolve-engines=false`.

Note: `engines.node` can be any semver range, with the latest satisfying version being resolved.
Note 2: If you disable it, please open an issue on GitHub describing _why_ you disabled it.
In the future, disabling it might be a no-op, so it's worth knowing any reason to
do that.

[env: FNM_RESOLVE_ENGINES]
[possible values: true, false]

-h, --help
Print help (see a summary with '-h')
Expand Down Expand Up @@ -782,12 +837,17 @@ Options:

[env: FNM_COREPACK_ENABLED]

--resolve-engines
--resolve-engines [<RESOLVE_ENGINES>]
Resolve `engines.node` field in `package.json` whenever a `.node-version` or `.nvmrc` file is not present.
Experimental: This feature is subject to change.
This feature is enabled by default. To disable it, provide `--resolve-engines=false`.

Note: `engines.node` can be any semver range, with the latest satisfying version being resolved.
Note 2: If you disable it, please open an issue on GitHub describing _why_ you disabled it.
In the future, disabling it might be a no-op, so it's worth knowing any reason to
do that.

[env: FNM_RESOLVE_ENGINES]
[possible values: true, false]

-h, --help
Print help (see a summary with '-h')
Expand All @@ -798,7 +858,7 @@ Options:
```
Uninstall a Node.js version

> Warning: when providing an alias, it will remove the Node version the alias is pointing to, along with the other aliases that point to the same version.
> Warning: when providing an alias, it will remove the Node version the alias > is pointing to, along with the other aliases that point to the same version.

Usage: fnm uninstall [OPTIONS] [VERSION]

Expand Down Expand Up @@ -845,12 +905,17 @@ Options:

[env: FNM_COREPACK_ENABLED]

--resolve-engines
--resolve-engines [<RESOLVE_ENGINES>]
Resolve `engines.node` field in `package.json` whenever a `.node-version` or `.nvmrc` file is not present.
Experimental: This feature is subject to change.
This feature is enabled by default. To disable it, provide `--resolve-engines=false`.

Note: `engines.node` can be any semver range, with the latest satisfying version being resolved.
Note 2: If you disable it, please open an issue on GitHub describing _why_ you disabled it.
In the future, disabling it might be a no-op, so it's worth knowing any reason to
do that.

[env: FNM_RESOLVE_ENGINES]
[possible values: true, false]

-h, --help
Print help (see a summary with '-h')
Expand Down
4 changes: 2 additions & 2 deletions e2e/env.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ for (const shell of [Bash, Zsh, Fish, PowerShell, WinCmd]) {
.then(
shell.redirectOutput(shell.call("fnm", ["env", "--json"]), {
output: filename,
})
}),
)
.takeSnapshot(shell)
.execute(shell)
Expand All @@ -26,7 +26,7 @@ for (const shell of [Bash, Zsh, Fish, PowerShell, WinCmd]) {
FNM_LOGLEVEL: "info",
FNM_MULTISHELL_PATH: expect.any(String),
FNM_NODE_DIST_MIRROR: expect.any(String),
FNM_RESOLVE_ENGINES: "false",
FNM_RESOLVE_ENGINES: "true",
FNM_COREPACK_ENABLED: "false",
FNM_VERSION_FILE_STRATEGY: "local",
})
Expand Down
2 changes: 1 addition & 1 deletion rust-toolchain.toml
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
[toolchain]
channel = "1.78"
channel = "1.81"
components = ["rustfmt", "clippy"]
2 changes: 1 addition & 1 deletion src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ pub enum SubCommand {
/// Uninstall a Node.js version
///
/// > Warning: when providing an alias, it will remove the Node version the alias
/// is pointing to, along with the other aliases that point to the same version.
/// > is pointing to, along with the other aliases that point to the same version.
#[clap(name = "uninstall", bin_name = "uninstall", visible_aliases = &["uni"])]
Uninstall(commands::uninstall::Uninstall),
}
Expand Down
5 changes: 4 additions & 1 deletion src/commands/ls_remote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,10 @@ pub struct LsRemote {

/// Show only LTS versions (optionally filter by LTS codename)
#[arg(long)]
#[allow(clippy::option_option)]
#[expect(
clippy::option_option,
reason = "clap Option<Option<T>> supports --x and --x=value syntaxes"
)]
Comment on lines +16 to +19
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related?

lts: Option<Option<String>>,

/// Version sorting order
Expand Down
16 changes: 12 additions & 4 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,16 +77,24 @@ pub struct FnmConfig {
corepack_enabled: bool,

/// Resolve `engines.node` field in `package.json` whenever a `.node-version` or `.nvmrc` file is not present.
/// Experimental: This feature is subject to change.
/// This feature is enabled by default. To disable it, provide `--resolve-engines=false`.
///
/// Note: `engines.node` can be any semver range, with the latest satisfying version being resolved.
/// Note 2: If you disable it, please open an issue on GitHub describing _why_ you disabled it.
/// In the future, disabling it might be a no-op, so it's worth knowing any reason to
/// do that.
#[clap(
long,
env = "FNM_RESOLVE_ENGINES",
global = true,
hide_env_values = true,
verbatim_doc_comment
)]
resolve_engines: bool,
#[expect(
clippy::option_option,
reason = "clap Option<Option<T>> supports --x and --x=value syntaxes"
)]
resolve_engines: Option<Option<bool>>,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it Option<Option<bool>> and not Option<bool>?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because Option<Option<bool>> means that --resolve-engines will work and so will --resolve-engines=false

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh

So --resolve-engines is Some(None), and --resolve-engines=false is Some(Some(false))?


#[clap(skip)]
directories: Directories,
Expand All @@ -102,7 +110,7 @@ impl Default for FnmConfig {
arch: Arch::default(),
version_file_strategy: VersionFileStrategy::default(),
corepack_enabled: false,
resolve_engines: false,
resolve_engines: None,
directories: Directories::default(),
}
}
Expand All @@ -118,7 +126,7 @@ impl FnmConfig {
}

pub fn resolve_engines(&self) -> bool {
self.resolve_engines
self.resolve_engines.flatten().unwrap_or(true)
}

pub fn multishell_path(&self) -> Option<&std::path::Path> {
Expand Down
4 changes: 2 additions & 2 deletions src/remote_node_index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ pub struct IndexedNodeVersion {
pub version: Version,
#[serde(with = "lts_status")]
pub lts: Option<String>,
pub date: chrono::NaiveDate,
pub files: Vec<String>,
// pub date: chrono::NaiveDate,
// pub files: Vec<String>,
}

/// Prints
Expand Down
Loading