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

build(tools): Add documenation for supporting tool version managers like asdf and mise-en-place #2505

Merged
merged 4 commits into from
Jan 7, 2025

Conversation

brandonlenz
Copy link
Contributor

@brandonlenz brandonlenz commented Jul 21, 2023

Summary

Nodenv is a great tool and many folks in the JS ecosystem use it. We support and recommend it today, and include a .node-version to help with that.

mise and asdf are also very popular, but typically more for developers working in a mixed tech stack (JS & Go, JS & Java, etc).

asdf and mise users likely need those tools for other projects, and should not be expected to switch to nodenv

It is also not recommended to use multiple node version managers, so either only asdf or only nodenv/n/nvm is recommended. Otherwise the tools can compete, causing unexpected shimming of Node to your PATH.

This PR preserves support for Nodenv for the folks who use that tool, but describes how to use asdf and mise for folks who use those tools. It is up to contributors to install, use, and manage the tool of their preference.

How To Test

As an asdf user, I tested by adding legacy_version_file, attempting to install the node version specified by the project. It then recommended I install the version from the .node-version file, which I did. Then I verified the installed version came up when within the react-uswds directory:

% node --version
> v18.15.0

@brandonlenz brandonlenz marked this pull request as ready for review July 21, 2023 15:25
@brandonlenz brandonlenz requested a review from a user July 21, 2023 15:25
@@ -25,7 +25,9 @@ We welcome contributions in the form of comments, issues, or pull requests with

## Environment setup

1. Use the node environment manager of your choice, but make sure you have the required version specified in `.node-version`. We recommend using [nodenv](https://github.com/nodenv/nodenv) to manage your node versions, but you can also use [homebrew](https://brew.sh/). More info can be found here: [how to install Node.js](https://nodejs.dev/how-to-install-nodejs)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The nodejs link was a 404, so I removed it.

Additionally, it is not recommended to use brew for Node, as brew prefers to use latest whenever possible. Contributors using brew will likely continue to use it (and hopefully know how to deal with those quirks), so I think it best to recommend a version manager and leave it at that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Something I've wondered for a while about asdf is that it supports .node-version file, is it better to have two files or just let asdf users know how to enable legacy_version_file for asdf?

https://github.com/asdf-vm/asdf-nodejs#nvmrc-and-node-version-support

Copy link
Contributor

Choose a reason for hiding this comment

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

@gidjin did y'all ever chat about this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We did, I never tested with the legacy_version_file enabled. I do need to do that. If that works, I can revert much of this PR, but still want the other doc changes/fixes to go into effect

Copy link
Contributor Author

Choose a reason for hiding this comment

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

While I'm still on asdf, I do plan on switching to Mise which supports .tool-versions, and .node-version, so this should be fine for that tool as well.

I just updated the PR for both cases

Copy link
Contributor

@gidjin gidjin left a comment

Choose a reason for hiding this comment

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

Left a note that I'd like to chat about before approving, otherwise this is good.

@brandonlenz brandonlenz requested a review from a team as a code owner March 8, 2024 19:32
@brandonlenz brandonlenz changed the title build(tools): Add support for asdf build(tools): Add documenation for supporting tool version managers like asdf and mise-en-place Jan 6, 2025
@brandonlenz brandonlenz enabled auto-merge (squash) January 6, 2025 15:05
@brandonlenz brandonlenz merged commit e4542dc into main Jan 7, 2025
9 checks passed
@brandonlenz brandonlenz deleted the bl-support-asdf branch January 7, 2025 00:34
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

Successfully merging this pull request may close these issues.

3 participants