Conversation
The VS Code extension telemetry needs a application insights as a side dependency.
There was a problem hiding this comment.
Pull request overview
Updates the extension’s dependency/tooling stack (VS Code engine, build/lint/test toolchain) and makes a few small TypeScript/code-quality adjustments, along with development documentation tweaks.
Changes:
- Bump VS Code engine requirement and update a broad set of npm dependencies/dev tools (ESLint/TypeScript/esbuild/sinon, etc.).
- Minor TypeScript refactors/fixes (settings
configuration.getcall, version parsing variable typing, richer Error cause in tests). - Refresh development docs and add repo-specific Copilot guidance for dependency updates.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
package.json |
Updates VS Code engine, runtime deps, and optional lint/test toolchain versions. |
package-lock.json |
Regenerates lockfile to reflect updated dependency graph. |
src/settings.ts |
Removes redundant generic argument on WorkspaceConfiguration.get. |
src/platform.ts |
Tweaks version parsing local variable typing. |
test/utils.ts |
Adds Error cause when binary module mock build fails. |
docs/development.md |
Updates upstream dependency tracking links and minor formatting. |
.github/copilot-instructions.md |
Adds guidance for updating npm packages in this repo. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "@types/sinon": "^21.0.1", | ||
| "@types/ungap__structured-clone": "^1.2.0", | ||
| "@types/vscode": "~1.101.0", | ||
| "@types/vscode": "~1.110.0", |
There was a problem hiding this comment.
@types/vscode is not aligned with the extension engines.vscode version. docs/development.md states @types/vscode must match the engine field when it’s updated; with engines.vscode at ^1.114.0, the @types/vscode entry should be updated to the corresponding ~1.114.0 (or the engine version adjusted to match the chosen types version).
| "@types/vscode": "~1.110.0", | |
| "@types/vscode": "~1.114.0", |
| When our `engine` field is updated the development dependency `@types/vscode` | ||
| must be updated to match. Note that it uses `~` (not `^`) so as to accept new | ||
| patches with `npm update` but not new minor versions. Then we check that version | ||
| of VS Code's own `package.json` file for their [`electron`][] dependency. The | ||
| major version of [Electron][] will tell us which [Node.js][] is included, which | ||
| dictates which version of Node.js the extension is eventually run with. This | ||
| lets us finally update our `@types/node` development dependency to match, our | ||
| developer machines if necessary, the CI and OneBranch pipeline tasks, and the | ||
| `.tsconfig` file. Note that the version of `@types/node` will not necessarily | ||
| exactly match the version of Node.js, but the major version should. | ||
|
|
||
| [`electron`]: https://github.com/microsoft/vscode/blob/ac4cbdf48759c7d8c3eb91ffe6bb04316e263c57/package.json#L163 | ||
| [Electron]: https://releases.electronjs.org/release/v37.7.0 | ||
| [Node.js]: https://nodejs.org/en/blog/release/v22.20.0 | ||
| [`electron`]: https://github.com/microsoft/vscode/blob/release/1.114/package.json | ||
| [Electron]: https://releases.electronjs.org/release/v39.8.3 | ||
| [Node.js]: https://nodejs.org/en/blog/release/v22.22.1 |
There was a problem hiding this comment.
This doc section says @types/vscode must match the engine field; however the updated VS Code reference now points at 1.114 while package.json still pins @types/vscode to ~1.110.0. Update the dependency (or the engine) so the documentation and package versions are consistent.
| "onCommand:PowerShell.SpecifyScriptArgs" | ||
| ], | ||
| "dependencies": { | ||
| "@vscode/extension-telemetry": "^1.5.0", | ||
| "@microsoft/applicationinsights-common": "^3.4.1", | ||
| "@vscode/extension-telemetry": "^1.5.1", | ||
| "semver": "^7.7.4", | ||
| "untildify": "^4.0.0", |
There was a problem hiding this comment.
PR metadata indicates this PR replaces #5449 (which fixed a TelemetryReporter default import in src/session.ts). That change does not appear to be included here (the default import remains), so if the goal is to carry forward #5449’s static-analysis fixes, this PR may still leave tsc --noEmit failing. Consider porting the src/session.ts telemetry import fix into this PR or updating the PR description to reflect the actual scope.
Replaces #5449