Skip to content

Conversation

andrewbranch
Copy link
Member

Fixes #62200

  • Adds deprecation message
  • Changes default moduleResolution for --module commonjs to bundler
  • Updates tests not relying on explicit node10 behavior
  • Fixes a module specifier completions bug affecting everything but node10

@Copilot Copilot AI review requested due to automatic review settings August 26, 2025 23:10
@github-project-automation github-project-automation bot moved this to Not started in PR Backlog Aug 26, 2025
@typescript-bot typescript-bot added Author: Team For Milestone Bug PRs that fix a bug with a specific milestone labels Aug 26, 2025
@typescript-bot
Copy link
Collaborator

Looks like you're introducing a change to the public API surface area. If this includes breaking changes, please document them on our wiki's API Breaking Changes page.

Also, please make sure @DanielRosenwasser and @RyanCavanaugh are aware of the changes, just as a heads up.

@typescript-bot
Copy link
Collaborator

Thanks for the PR! It looks like you've changed the TSServer protocol in some way. Please ensure that any changes here don't break consumers of the current TSServer API. For some extra review, we'll ping @sheetalkamat, @mjbvz, @zkat, and @joj for you. Feel free to loop in other consumers/maintainers if necessary.

@zkat
Copy link
Contributor

zkat commented Aug 26, 2025

Looks like something you might want to stop pinging me on btw. Hope all is well! ❤️

@andrewbranch
Copy link
Member Author

@zkat I was just working on that 😄❤️ Same to you!

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@jakebailey
Copy link
Member

Before we do this we'll need to update DT to support non node10 resolution :(

@andrewbranch
Copy link
Member Author

I'm not sure what you mean; AFAICT nothing in DT relies on node10. You’re never allowed to specify moduleResolution; instead, it’s enforced that module is either node16 or commonjs, and in the latter case the default module resolution will silently swap from node10 to bundler, which is what we want. We should just make sure, ahead of the 6.0 release, that we handle any breakages that causes.

@andrewbranch
Copy link
Member Author

656 of 9071 currently use --module commonjs. For most of them, IIRC that means a few years ago they couldn’t be automatically migrated to node16. bundler should be an easier migration for them, though there could still be errors from misconfigured upstream package.json "exports".

@jakebailey
Copy link
Member

Yeah, you're right.

@github-project-automation github-project-automation bot moved this from Not started to Needs merge in PR Backlog Aug 27, 2025
@andrewbranch andrewbranch merged commit 7956c00 into microsoft:main Aug 27, 2025
33 checks passed
@github-project-automation github-project-automation bot moved this from Needs merge to Done in PR Backlog Aug 27, 2025
@sheetalkamat
Copy link
Member

I think we also need to change resolution mode in resolveLibrary options ? https://github.com/microsoft/TypeScript/blob/main/src/compiler/moduleNameResolver.ts#L1388C32-L1388C52

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Milestone Bug PRs that fix a bug with a specific milestone
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Deprecate, remove --moduleResolution node10
6 participants