Complete native ESM migration - #9618
Closed
dibarbet wants to merge 1 commit into
Closed
Conversation
dibarbet
force-pushed
the
dibarbet-esm-nodenext-conversion
branch
from
August 4, 2026 01:10
d9a568d to
61309d9
Compare
davidwengier
approved these changes
Aug 4, 2026
Audit the remaining CommonJS bundle bridge, prefer the telemetry ESM entry, and validate the native ESM release artifact. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 89dc5364-d759-419b-b4e5-afa454dfcd41
dibarbet
force-pushed
the
dibarbet-esm-migration-cleanup
branch
from
August 4, 2026 01:56
dd53025 to
96639c0
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR completes the extension’s native ESM migration by upgrading @vscode/extension-telemetry, ensuring the bundled entry is the package’s ESM module, and adding automated audits/tests to prevent regressions (CommonJS bridge growth, activation/export contract, signing, and VSIX inclusion).
Changes:
- Upgrades
@vscode/extension-telemetryto^1.5.2and updates activation code to use the ESM-oriented import path. - Adds an esbuild metafile-based bundle audit (with allowlist + stale detection) and wires it into the esbuild pipeline.
- Adds artifact tests to validate
.mjsentry existence/exports, signing coverage, and VSIX packaging of the declared entry.
Show a summary per file
| File | Description |
|---|---|
| test/tasks/bundleAudit.test.js | Adds a unit test covering bundle audit failure on first-party CommonJS runtime require usage. |
| test/omnisharp/omnisharpUnitTests/logging/telemetryObserver.test.ts | Updates telemetry observer unit test to align with the updated error-event signature. |
| test/lsptoolshost/artifactTests/vsix.test.ts | Enhances VSIX artifact validation to ensure the declared extension entry is present. |
| test/lsptoolshost/artifactTests/extensionBundle.test.ts | Adds artifact-level checks for .mjs entry existence, activation export contract, and signing coverage. |
| test/fakes.ts | Updates telemetry reporter fake to match the updated error-event API shape. |
| tasks/compilation/bundleAudit.mjs | Introduces the runtime-require audit for bundled CommonJS dependencies (allowlist + drift detection). |
| src/shared/telemetryReporter.ts | Updates the telemetry reporter interface to remove the unused errorProps parameter. |
| src/omnisharp/observers/telemetryObserver.ts | Stops passing errorProps to telemetry (documenting it was ignored upstream). |
| src/main.ts | Switches telemetry import/usage to match the upgraded telemetry package. |
| package.json | Bumps @vscode/extension-telemetry dependency to ^1.5.2. |
| package-lock.json | Updates lockfile for telemetry upgrade and transitive dependency changes. |
| esbuild.mjs | Adds resolution override for telemetry ESM entry and wires in bundle audit via esbuild metafile. |
Copilot's findings
- Files reviewed: 11/12 changed files
- Comments generated: 2
| import { EventStream } from './eventStream.ts'; | ||
| import { PlatformInformation } from './shared/platform.ts'; | ||
| import telemetryReporterModule from '@vscode/extension-telemetry'; | ||
| import { TelemetryReporter } from '@vscode/extension-telemetry'; |
Comment on lines
+28
to
+35
| const entries: string[] = []; | ||
| zipFile.on('entry', (entry) => { | ||
| entries.push(entry.fileName); | ||
| zipFile.readEntry(); | ||
| }); | ||
| zipFile.on('end', () => resolve(entries)); | ||
| zipFile.on('error', reject); | ||
| zipFile.readEntry(); |
Member
Author
|
Superseded by #9633 on |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
@vscode/extension-telemetryfrom 0.9.0 to 1.5.2 and bundle its advertised ESM entry.mjsactivation contract, signing coverage, and VSIX inclusion.tssource specifiers andrewriteRelativeImportExtensionsBundle audit
require("vscode")remains only inmicrosoft.aspnetcore.razor.vscodeandvscode-languageclientcreateRequirebridge remains for audited CommonJS dependencies that require Node built-insValidation
--fixnpm run compileDevnpm run test:unitnpm run packagenpm run test:artifactsnpm run test:integration:untrustedextension/dist/extension.mjsnode --check dist/extension.mjsgit diff --check