-
Notifications
You must be signed in to change notification settings - Fork 96
fix(vscode): Updating Dependencies and fixing Azurite #8729
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
base: main
Are you sure you want to change the base?
Conversation
…re azurite files is sometimes incorrectly put into the workspace
🤖 AI PR Validation ReportPR Review ResultsThank you for your submission! Here's detailed feedback on your PR title and body compliance:✅ PR Title
✅ Commit Type
❌ Risk Level
❌ What & Why
|
| Section | Status | Recommendation |
|---|---|---|
| Title | ✅ | Consider specifying versions bumped and Azurite fix |
| Commit Type | ✅ | No change needed |
| Risk Level | ❌ | Add a risk:* label and update PR body to Medium or justify Low |
| What & Why | ❌ | Add a brief description of what changed and why (see example above) |
| Impact of Change | Fill out Users/Developers/System with specific impacts and migration notes | |
| Test Plan | ✅ | Add reproduction steps for manual testing and E2E test results or rationale |
| Contributors | Add contributors or a short note if none | |
| Screenshots/Videos | ✅ | N/A |
Final message
Please update the PR title/body with the items above and re-submit:
- Add a short "What & Why" describing the dependency bumps (Node 18 -> 20, .NET 6 -> 8) and the Azurite workspace fix.
- Add explicit impact bullets for Users/Developers/System.
- Add/confirm the risk label. I recommend
risk:mediumgiven the runtime upgrades — either change the PR risk selection to Medium and add therisk:mediumlabel, or provide a clear justification why this is Low risk. - Expand the Test Plan with manual test steps and environment details and confirm whether E2E tests were run (the diff changes e2e setup to Node 20).
- Add a short contributors list (or note none).
Once those updates are made, re-request review. Thanks — the code diffs and tests look good, but the PR metadata needs completion and the risk should be adjusted/justified.
(Note: advised risk is higher than the one selected in the PR body — advised: risk:medium vs selected: Low.)
Last updated: Sat, 24 Jan 2026 01:31:46 GMT
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.
Pull request overview
This PR updates the VS Code extension's Node.js and .NET dependencies from legacy versions to current supported versions, and fixes an Azurite configuration issue where files could be incorrectly placed in the workspace folder.
Changes:
- Updates Node.js dependency from v18 to v20 across runtime configuration, fallback versions, and E2E test infrastructure
- Updates .NET dependency from v6 to v8 for SDK fallback versions
- Fixes Azurite location setting to use a fallback default path when the extension setting is undefined, preventing incorrect workspace folder usage
- Adds telemetry logging for the new Node.js fallback path when no matching version is found
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| e2e/testSetup/BlankLogicApp/logicAppStandard.bicep | Updates WEBSITE_NODE_DEFAULT_VERSION from ~18 to ~20 for Azure App Service configuration |
| apps/vs-code-designer/src/constants.ts | Updates workflowappRuntime to node|20, renames dotnet6 to dotnet8 with version 8.0.318, and updates Node.js fallback to 20.18.3 |
| apps/vs-code-designer/src/app/utils/binaries.ts | Updates all .NET 6 references to .NET 8, adds fallback logic with telemetry when no matching Node.js version is found |
| apps/vs-code-designer/src/app/utils/azurite/activateAzurite.ts | Fixes Azurite location setting to fall back to default path instead of undefined/empty value |
| apps/vs-code-designer/src/app/utils/test/binaries.test.ts | Updates test expectations from dotnet6 to dotnet8 |
| context.telemetry.properties.latestNodeJSVersion = 'fallback-no-match'; | ||
| context.telemetry.properties.errorLatestNodeJsVersion = 'No matching Node JS version found.'; | ||
| return DependencyVersion.nodeJs; |
Copilot
AI
Jan 23, 2026
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.
The new fallback path when no matching Node.js version is found (lines 241-243) is not covered by unit tests. The existing test at line 237 only covers the API failure case (catch block), but doesn't test the scenario where the API succeeds but returns releases that don't match the requested major version. Consider adding a test case that mocks a successful API response with releases that don't match the major version to verify this new fallback behavior works correctly.
Updating node dependency to 20 and from net6 to net8, fixed issue where azurite files is sometimes incorrectly put into the workspace
Commit Type
Risk Level
What & Why
Impact of Change
Users should now see node being downloaded properly.
Test Plan
Contributors
Screenshots/Videos