feat(migrate): migrate .nvmrc to .node-version during vp migrate#1159
feat(migrate): migrate .nvmrc to .node-version during vp migrate#1159naokihaba wants to merge 7 commits intovoidzero-dev:mainfrom
vp migrate#1159Conversation
… to .node-version
✅ Deploy Preview for viteplus-preview canceled.
|
| async function confirmNodeVersionFileMigration(interactive: boolean): Promise<boolean> { | ||
| if (interactive) { | ||
| const confirmed = await prompts.confirm({ | ||
| message: 'Migrate .nvmrc to .node-version?', |
There was a problem hiding this comment.
In Vite+, the Node.js version is determined based on the following priority order:
The sequence is ".node-version", followed by "engines.node" in package.json, and then "devEngines.runtime" in package.json.
For this reason, we are treating the migration of the ".node-version" file as the highest priority in this migration process.
vite-plus/crates/vite_global_cli/src/commands/env/config.rs
Lines 197 to 207 in d2e4dfd
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d2e4dfd821
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Hmm... The snapshot diff is failing.
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db3da182a1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| * Returns null for unsupported aliases like "node", "stable", "system". | ||
| */ | ||
| export function parseNvmrcVersion(content: string): string | null { | ||
| const version = content.split('\n')[0]?.trim(); |
There was a problem hiding this comment.
Parse
.nvmrc using full nvm syntax before migration
parseNvmrcVersion only inspects the first line (content.split('\n')[0]), so valid .nvmrc files that start with a comment/blank line or include inline comments/key-value lines are treated as unsupported and migration is skipped with a warning. Per nvm’s .nvmrc docs, comments and blank lines should be ignored during parsing (see https://github.com/nvm-sh/nvm#nvmrc), so this can silently miss real-world projects and fail the new auto-migration path even when a valid version is present later in the file.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The existing process for reading .node-version (parse_node_version_content) is also designed to read only the first line without excluding comments.
Therefore, this implementation follows the same limitations to maintain consistency across the codebase.
If necessary, I can revisit this and make further adjustments.
🔗 Related Issues
Resolves: #1158
📚 Description
Vite+ previously retrieved the Node.js version from .node-version, but projects using nvm typically use .nvmrc.
With this change, automatic detection of .nvmrc and a migration feature to .node-version have been added as part of the vp migrate command execution process.
During migration, if a v prefix is present at the beginning, it is removed, while aliases such as lts/* are carried over as is.
Unit tests have confirmed that it works correctly regardless of the presence of the v prefix.
On the other hand, if unsupported aliases like node or stable are included, the system is designed to display a warning and skip the migration rather than failing.
Manual Test: .nvmrc Migration
Command
Output (Excerpt)
Final State