Skip to content

Complete native ESM migration - #9633

Closed
dibarbet wants to merge 1 commit into
dev/esm-nodenext-conversionfrom
dev/esm-migration-cleanup
Closed

Complete native ESM migration#9633
dibarbet wants to merge 1 commit into
dev/esm-nodenext-conversionfrom
dev/esm-migration-cleanup

Conversation

@dibarbet

@dibarbet dibarbet commented Aug 4, 2026

Copy link
Copy Markdown
Member

Summary

  • replace Complete native ESM migration #9618 on the rebuilt dev/* stack, targeting replacement layer 3 from Convert TypeScript sources to NodeNext ESM #9632
  • inherit direct TypeScript emission of out/test/jestProjectNames.mjs via allowJs and narrow compiler inputs; no copy task or post-tsc copy step remains
  • upgrade @vscode/extension-telemetry to 1.5.2 and explicitly select its advertised ESM entry
  • add an exact esbuild metafile audit for the remaining createRequire compatibility bridge
  • validate the native .mjs activation, signing, and VSIX artifact contract

Dev CI validates this replacement chain against dev/esm-nodenext-conversion; the layer-4 replay retains the same stable patch ID as old layer commit 96639c04.

Bundle audit

  • development and production: 46 owners, 279 modules, 378 runtime requires
  • require("vscode") remains limited to microsoft.aspnetcore.razor.vscode and vscode-languageclient
  • no audit baseline update was needed

Validation

  • targeted ESLint with --fix
  • npm run compileDev
  • direct tsc support-file audit: out/test/jestProjectNames.mjs exists and no Jest configs are emitted
  • npm run test:unit (12 suites, 515 tests)
  • npm run package
  • explicit development and production bundle audits
  • npm run test:artifacts (2 suites, 7 tests)
  • VSIX inspection: extension/package.json declares ./dist/extension.mjs, extension/dist/extension.mjs is present, and legacy extension.js is absent
  • npm run test:integration:untrusted (1 suite, 1 test)
  • full npm run test:integration:csharp (21 suites; 73 passed, 3 skipped)
  • node --check dist/extension.mjs
  • git diff --check, stable patch-id, and git range-diff

@dibarbet
dibarbet requested a review from a team as a code owner August 4, 2026 21:22
@dibarbet
dibarbet force-pushed the dev/esm-nodenext-conversion branch from 26b5aed to c6cd3b8 Compare August 4, 2026 22:56
@dibarbet
dibarbet requested a review from a team as a code owner August 4, 2026 22:56
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c66ed411-9132-42f1-8a24-83a5aa049ab3
Copilot AI review requested due to automatic review settings August 4, 2026 23:15
@dibarbet
dibarbet force-pushed the dev/esm-migration-cleanup branch from 144c03e to 739dd57 Compare August 4, 2026 23:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 completes the extension’s native ESM migration by upgrading telemetry to an ESM-capable version, enforcing bundle/runtime-require invariants via an esbuild metafile audit, and tightening artifact validation to ensure the VSIX and bundle activation contract align with the .mjs entrypoint.

Changes:

  • Upgrade @vscode/extension-telemetry to ^1.5.2 and switch to the package’s ESM TelemetryReporter export.
  • Add a bundle audit (auditBundle) and wire it into esbuild.mjs to enforce a stable/allowlisted createRequire bridge surface.
  • Add/extend Jest artifact tests to validate the .mjs entrypoint, signing coverage, and VSIX contents.
Show a summary per file
File Description
test/tasks/bundleAudit.test.js Adds a unit test ensuring the new bundle audit rejects first-party CommonJS runtime requires.
test/omnisharp/omnisharpUnitTests/logging/telemetryObserver.test.ts Updates telemetry observer tests to align with the new error-event reporting signature (no errorProps).
test/lsptoolshost/artifactTests/vsix.test.ts Extends VSIX artifact validation to ensure the declared extension entrypoint exists inside the VSIX.
test/lsptoolshost/artifactTests/extensionBundle.test.ts Adds artifact tests validating .mjs entrypoint, activation export contract, and signing includes .mjs.
test/fakes.ts Updates telemetry fakes to the updated sendTelemetryErrorEvent signature.
tasks/compilation/bundleAudit.mjs Introduces a metafile-based runtime-require audit with an allowlist and activation-contract checks.
src/shared/telemetryReporter.ts Updates the telemetry reporter interface signature to remove errorProps.
src/omnisharp/observers/telemetryObserver.ts Stops passing errorProps to telemetry and documents the behavior change.
src/main.ts Switches to named TelemetryReporter import (ESM) instead of CommonJS default-wrapper logic.
package.json Bumps @vscode/extension-telemetry dependency to ^1.5.2.
package-lock.json Updates lockfile to reflect the telemetry upgrade and transitive dependency changes.
esbuild.mjs Adds an ESM-resolve plugin for telemetry and enforces the new bundle audit during builds via an esbuild plugin.

Copilot's findings

  • Files reviewed: 11/12 changed files
  • Comments generated: 2

Comment on lines +69 to +79
function getPackageOwner(inputPath) {
const normalizedPath = inputPath.replaceAll('\\', '/');
const marker = 'node_modules/';
const markerIndex = normalizedPath.indexOf(marker);
if (markerIndex < 0) {
return undefined;
}

const [firstSegment, secondSegment] = normalizedPath.slice(markerIndex + marker.length).split('/');
return firstSegment.startsWith('@') ? `${firstSegment}/${secondSegment}` : firstSegment;
}
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();
@dibarbet

dibarbet commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

Superseded by #9635, which consolidates the complete native ESM migration into one final-state commit and removes intermediate workaround history.

@dibarbet dibarbet closed this Aug 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants