Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use ESM for our executables (tsc, tsserver, typingsInstaller) #51440

Open
jakebailey opened this issue Nov 8, 2022 · 11 comments
Open

Use ESM for our executables (tsc, tsserver, typingsInstaller) #51440

jakebailey opened this issue Nov 8, 2022 · 11 comments
Assignees
Labels
Infrastructure Issue relates to TypeScript team infrastructure

Comments

@jakebailey
Copy link
Member

jakebailey commented Nov 8, 2022

Our libraries are all UMD-ish, because they can be used in Node or in the browser. I didn't change this for the module transform.

However, most of our executables are CJS, as they are only intended to be run within Node. This includes tsc, typingsInstaller, and watchGuard. An exception is tsserver, which contains web code for vscode.dev.

tsc and typingsInstaller both share a lot of code; if they only run on Node anyway, we could potentially raise our minimum Node version to node 12, and ship them as ESM via esbuild's split ESM bundling. If we get vscode.dev off of tsserver (and onto tsserverlibrary), then all three can share code.

In my testing, this reduces our package size by an additional 7 MB, which is a great win.

See also #27891, #32949.

@jakebailey jakebailey self-assigned this Nov 8, 2022
@fatcerberus
Copy link

if they only run on Node anyway, we could potentially raise our minimum Node version to node 12, and ship them as ESM

Isn't Node 14 the earliest version that supports ESM without a command-line flag?

@jakebailey
Copy link
Member Author

jakebailey commented Nov 8, 2022

Isn't Node 14 the earliest version that supports ESM without a command-line flag?

Nope, it's enabled by default in Node 12.17+.

But, I would want to target Node 12.20+ as that's when the experimental warning was removed. Technically, Node 12.22 is when it's "stable", but AFAICT 12.20 works fine.

@jakebailey
Copy link
Member Author

Another totally radical idea would be to just ship ESM now with separate endpoints. It wouldn't behave how people want, as in no tree shaking, but it wouldn't be that much code. tsserver already pulls pretty much everything in anyway, so this wouldn't be all that much extra.

We'd just still have the dual package hazard, and have to get the weirdness of export mappings and self references.

@DanielRosenwasser DanielRosenwasser added the Infrastructure Issue relates to TypeScript team infrastructure label Nov 8, 2022
@lemanschik

This comment was marked as off-topic.

@lemanschik

This comment was marked as off-topic.

@lemanschik

This comment was marked as off-topic.

@jakebailey
Copy link
Member Author

With #51699 merged, tsserver can now safely be ESM as the web code has been entirely switched to tsserverlibrary.

That being said, I still haven't quite been able to get the codebase to run in esbuild's code splitting mode, probably due to ordering problems thanks to our codebase's continued circularity.

@lemanschik

This comment was marked as off-topic.

@DanielRosenwasser

This comment was marked as off-topic.

@lemanschik

This comment was marked as off-topic.

@jakebailey
Copy link
Member Author

Mostly related, but I've successfully gotten the TS API itself to run as ESM while being required from CJS: nodejs/node#52599 (comment)

--experimental-require-module being unflagged with some solution to nodejs/node#52599 would let us just be ESM for everything.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Infrastructure Issue relates to TypeScript team infrastructure
Projects
None yet
Development

No branches or pull requests

4 participants