Make TypeScript itself ESM-only, made possible by require(ESM)#58419
Make TypeScript itself ESM-only, made possible by require(ESM)#58419jakebailey wants to merge 6 commits intomicrosoft:mainfrom
Conversation
|
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. |
9a1860f to
42acb3b
Compare
|
This has been rebased now that we're using extensions on main, condensing this down to a pretty small change (though there's still stuff not working). |
bed73cc to
739c9bf
Compare
|
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. |
|
There are still a bunch of TODOs, but this is actually green! Wow! |
18d1d83 to
423f584
Compare
|
nodejs/node#52762 was merged a bit ago; I should be able to retarget this PR at nightly. |
7c70c64 to
208e064
Compare
793228e to
4c232d5
Compare
e272d8f to
8a63274
Compare
|
@typescript-bot perf test this faster hosts=node@23.0.0 |
|
@jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Huh, where is the perf hit coming from here...? |
|
Given the Go port, I don't think it makes sense for me to attempt this anymore. I think it's only going to cause more churn in the meantime. |
nodejs/node#51977 (behind
--experimental-require-modulein Node 22) enables Node to require ESM so long as that ESM does not make use of top-level await. TypeScript would only need top-level await to constructts.sysat startup, which must be defined when the environment is detected to be Node. Via nodejs/node#52599 and nodejs/node#51977, we can synchronously access Node's built-ins, and therefore can offer up a TLA-free public API, enabling TypeScript to ship as ESM-only without breaking CJS users, oncerequire(ESM)is unflagged.TODO:
__esModulerequire(ESM)having__esModule. But I suspect that exportingdefaultfixes this.requireifprocess.getBuiltinModuleis not foundMaybe still ship CJS behind a condition to get this out sooner?Dual package hazard is very scary here. Maybe acceptable withmodulecondition/top-level prop?Stop bundling? Share code between tsc.js/typescript.js?@types/diffis typed wrong