-
Notifications
You must be signed in to change notification settings - Fork 592
Add support for configuring npm version in Node devcontainers #1616
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
Open
sireeshajonnalagadda
wants to merge
12
commits into
devcontainers:main
Choose a base branch
from
sireeshajonnalagadda:npm-version
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
b3aa446
feat(node): add npm version selection and installation options
sireeshajonnalagadda 895f1c8
Merge branch 'main' into npm-version
sireeshajonnalagadda f78d73f
add 'lts' and 'latest' options for npm version selection
sireeshajonnalagadda 9a0c859
feat(node): enhance npm installation with compatibility checks and fa…
sireeshajonnalagadda 89f5747
Merge branch 'main' into npm-version
sireeshajonnalagadda 29c6a4d
Update src/node/devcontainer-feature.json
sireeshajonnalagadda ad44844
Update src/node/devcontainer-feature.json
sireeshajonnalagadda 0eca8ad
feat(tests): enhance npm version checks for compatibility and fallbac…
sireeshajonnalagadda 5fcf2da
Version bump
sireeshajonnalagadda f23509b
Version bump
sireeshajonnalagadda 1e22b8c
Merge branch 'main' into npm-version
sireeshajonnalagadda 549ffdf
Merge branch 'main' into npm-version
sireeshajonnalagadda File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| #!/bin/bash | ||
|
|
||
| set -e | ||
|
|
||
| # Optional: Import test library | ||
| source dev-container-features-test-lib | ||
|
|
||
| # When npmVersion="latest", npm should be upgraded from Node.js bundled version if possible | ||
| # Node.js 22 comes with npm 10.x, latest should be 11+ if upgrade succeeds | ||
| # If upgrade fails, npm should still work (may remain at bundled version) | ||
| check "npm_version_upgraded_or_functional" bash -c " | ||
| npm --version >/dev/null | ||
| NPM_MAJOR=\$(npm --version | cut -d. -f1) | ||
|
|
||
| if [ \$NPM_MAJOR -ge 11 ]; then | ||
| echo 'npm successfully upgraded to version 11+ (\$NPM_MAJOR.x)' | ||
| exit 0 | ||
| elif [ \$NPM_MAJOR -eq 10 ]; then | ||
| echo 'npm upgrade may have failed, but npm 10.x is still functional' | ||
| exit 0 | ||
| else | ||
| echo 'npm version \$NPM_MAJOR.x - unexpected version' | ||
| exit 1 | ||
| fi | ||
| " | ||
|
|
||
| # Also verify pnpm works as configured | ||
| check "pnpm_version" bash -c "pnpm -v | grep 8.8.0" | ||
|
|
||
| # Report result | ||
| reportResults |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| #!/bin/bash | ||
|
|
||
| set -e | ||
|
|
||
| # Optional: Import test library | ||
| source dev-container-features-test-lib | ||
|
|
||
| # Test: npm "latest" with Node.js 16.x (incompatible scenario) | ||
| # Should show compatibility warning and auto-fallback to compatible version (npm 9.x) | ||
|
|
||
| # Verify we have Node.js 16.x as expected | ||
| check "node_version_16" bash -c "node -v | grep '^v16\.'" | ||
|
|
||
| # Check npm is functional after installation attempt | ||
| check "npm_works" bash -c "npm --version" | ||
|
|
||
| # Verify npm version fell back to compatible version for Node 16.x (should be npm 8.x) | ||
| check "npm_fallback_version" bash -c " | ||
| NPM_MAJOR=\$(npm --version | cut -d. -f1) | ||
| if [ \$NPM_MAJOR -eq 8 ]; then | ||
| echo 'npm auto-fell back to version 8.x (compatible with Node 16.x)' | ||
| exit 0 | ||
| else | ||
| echo 'npm version \$NPM_MAJOR.x - fallback may not have worked correctly' | ||
| exit 1 | ||
| fi | ||
| " | ||
|
|
||
| # Report result | ||
| reportResults |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| #!/bin/bash | ||
|
|
||
| set -e | ||
|
|
||
| # Optional: Import test library | ||
| source dev-container-features-test-lib | ||
|
|
||
| # When npmVersion is "none", npm should not be updated from node's bundled version | ||
| check "npm_not_updated" bash -c ' | ||
| npm --version >/dev/null | ||
|
|
||
| NODE_MAJOR=$(node -p "process.versions.node.split(\".\")[0]") | ||
| NPM_MAJOR=$(npm --version | cut -d. -f1) | ||
|
|
||
| case "$NODE_MAJOR" in | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we can control which Node version is installed, can we pin to the latest (24) -- so that upgrades don't break this test, and then verify NPM 11 is installed? |
||
| 16) EXPECTED_NPM_MAJOR=8 ;; | ||
| 18|20|22) EXPECTED_NPM_MAJOR=10 ;; | ||
| 24) EXPECTED_NPM_MAJOR=11 ;; | ||
| *) | ||
| echo "Unsupported Node major for bundled npm assertion: $NODE_MAJOR" | ||
| exit 1 | ||
| ;; | ||
| esac | ||
|
|
||
| [ "$NPM_MAJOR" = "$EXPECTED_NPM_MAJOR" ] | ||
| ' | ||
|
|
||
| # Report result | ||
| reportResults | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| #!/bin/bash | ||
|
|
||
| set -e | ||
|
|
||
| # Optional: Import test library | ||
| source dev-container-features-test-lib | ||
|
|
||
| # Verify npm is installed with specific version 10.8.0 | ||
| check "npm_specific_version" bash -c "npm -v | grep '^10.8.0'" | ||
|
|
||
| # Report result | ||
| reportResults |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please move these to the top condition? That would simplify the branching here a lot.