Skip to content

NPM-PACKAGE - Move to ESM#7441

Open
deinok wants to merge 25 commits intomicrosoft:mainfrom
deinok:main
Open

NPM-PACKAGE - Move to ESM#7441
deinok wants to merge 25 commits intomicrosoft:mainfrom
deinok:main

Conversation

@deinok
Copy link
Copy Markdown
Contributor

@deinok deinok commented Mar 3, 2026

@baywet Getting clean ESM + CommonJS outputs without introducing bundling has been messy, so I went with this approach:

  • Ship ESM only (removed the CommonJS build)
  • Fixed a few JSON import issues
  • Removed the JSON copy script (not needed since we import it directly)
  • Moved some files under the lib/ folder
  • Updated tsconfig.json to include lib/ instead of excluding everything else

The VSCode plugin should still work fine with ESM because it’s bundled with webpack, so webpack will bundle what it needs from the ESM module and tree-shake unused exports where possible.

Note: If this package is consumed outside the VSCode extension, switching to ESM-only may be a breaking change for environments still relying on CommonJS.

@deinok deinok marked this pull request as ready for review March 3, 2026 20:12
@deinok deinok requested a review from a team as a code owner March 3, 2026 20:12
Copy link
Copy Markdown
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution!

I double checked and the other big consumer of this package is also using a bundler
https://github.com/OfficeDev/microsoft-365-agents-toolkit/blob/c0eb0bed661202bfd85b3bb1bd61e21b0b73c0b4/packages/fx-core/package.json#L87

@deinok
Copy link
Copy Markdown
Contributor Author

deinok commented Mar 5, 2026

@baywet I left two points unresolved. Also could you please trigger CI?

@baywet
Copy link
Copy Markdown
Member

baywet commented Mar 6, 2026

@baywet I left two points unresolved. Also could you please trigger CI?

done. Another aspect of cjs I forgot to mention earlier is jest's configuration that'll need to be updated.

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