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

Roadmap for experimental TypeScript support #217

Open
3 of 5 tasks
marco-ippolito opened this issue Jul 15, 2024 · 30 comments
Open
3 of 5 tasks

Roadmap for experimental TypeScript support #217

marco-ippolito opened this issue Jul 15, 2024 · 30 comments
Labels
typescript Discussions related to TypeScript typescript-agenda Issue or PRs to be discussed during TypeScript team meeting

Comments

@marco-ippolito
Copy link
Member

marco-ippolito commented Jul 15, 2024

Intro

Since the previous discussion was successful in gathering feedback from the community, and I believe there is some consensus that this feature is something that the project wants to add, I want to summarize what are going to be the next steps and some technical solutions.
To be honest I'm very glad for the amount of feedback I received, it helped me to change my view on some aspects that I previously ignored.
Before I go into technical implementation, this is what the goals of this feature should be in my opinion:

Caution

These are long term goals and next steps, they might not reflect the current status.

  • Make sure Node.js can support TypeScript with the stability guarantees that has always offered.
  • Avoid breaking the ecosystem, this means creating a new "flavor" that only works on Node.js.
  • Performance.
  • Keeping it simple, we don't support everything, we want to user land tools to work in sync with Node, we provide the foundations to build on top.
  • Type stripping is the way to go.

Step 1: Initial implementation

The initial implementation is the proof of concept that I have created to gather feedback and consensus from the project collaborators.
It's very far from being perfect but it establishes some of the points we want to move forward with.
Current limitations:

  • No support forTypeScript features that require transformation (Enums, namespace, etc...).
  • No .js extension for .ts files.
  • No running TypeScript in node_modules.
  • No source maps, but not needed because we perform whitespacing (replace removed code with white space).

Important

Why no support for TypeScript features such as enums, namespaces, decorators, etc...?????

  • In the initial implementation I decided to start with the smallest subset possible.
  • Adding transformation means supporting source-maps so is more work.
  • Supporting features means adding more instability to the TypeScript syntax we support, without having a way to stay up to date with new features. As a general rule of thumb, the more you support the more likely to have breaking changes. This is a great challenge to overcome because we need to support transformations and new features but respect Node stability guarantees

Step 2: Decoupling

There is already a precedent for something that Node.js support, that can be upgraded seperately, its NPM.
Node bundles a version of npm that can upgraded separately, we could do the same with our TypeScript transpiler.

We could create a package that we bundle but that can also be downloaded from NPM, keep a stable version in core, but if TypeScript releases new features that we don't support or breaking changes, or users want to use the new shiny experimental feature, they can upgrade it separately.
This ensures that users are not locked, but also provides support for a TypeScript version for the whole 3 years of the lifetime of Node.js release.

Getting started

Create a new package called amaro that wraps swc with the current implementation. This package, which is released on npm, offers the same api that is currently used by Node, and it can be upgraded separately. The first challenge would be setup the project, make sure it can be upgraded by running npm install amaro@vX.Y.Z

Increase features

  • Support for transformation behind a flag

  • Enable support for TypeScript features that require transformation (Enums, mamespaces), now that we are decoupled we can expand on the amount of feature we support.

Caution

Currently there is no consensus whether Node.js should run TypeScript files inside `node_modules.
Currently it is not supported to avoid package maintainers to release TS only package.

Thanks @joyeecheung and @legendecas for the idea 💡

Step 3: Make it fast

  • TBD

With the project up and running, we can now start thinking about performance.
We could vendor SWC in Rust, build our own wasm, or compile it to static libraries that we could use in core, c++ rust FFI, etc...
I'm not a performance expert, but I think it is possible to optimize the interaction between Node and SWC and make it performant, without impacting the Node build process.
This is the phase where we measure the performance and make it usable in production without performance penalties.

Step 4: Add more features

  • TBD

Amaro in the current state does not offer much to users, it's released as an external package but it's pretty much useless other than as an internal.
In this phase we could add extra features that are not used in core, but provide utilities to users to reduce some of the pain that our restriction might have cause.
Example:

  • Offer a loader api that allows transformation .js => .ts that can be enabled with a flag or as an external loader.
  • Offer more configuration, tsconfig support etc...

Wont Fix

  • I strongly believe we should not support .js extension for .ts files.
    The reason is that the compiler/bundler should be responsible to resolve the correct extension at compile time.
    At runtime, the extension must be correct, it doesn't make sense to add overhead in production when it can be solved during development. Discussion happening here: Import specifiers in --experimental-strip-types #214
  • Support tsconfig directly
    This would mean to run with typescript to support the latest flavors etc... Compilation and type checking should be done by user land tools during development.
@marco-ippolito marco-ippolito added the typescript Discussions related to TypeScript label Jul 15, 2024
nodejs-github-bot pushed a commit to nodejs/node that referenced this issue Jul 24, 2024
PR-URL: #53725
Refs: nodejs/loaders#217
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruy Adorno <ruy@vlt.sh>
@GeoffreyBooth
Copy link
Member

I think this looks great; can we just replace the contents of the .md file in #210 with what you’ve written here? And then we can land that PR.

@silverwind
Copy link

silverwind commented Jul 25, 2024

No support forTypeScript features that require transformation (Enums, namespace, etc...).

Is there a definitive list of features that are not supported here? I'm looking to adapt my eslint config to match.

@marco-ippolito
Copy link
Member Author

No support forTypeScript features that require transformation (Enums, namespace, etc...).

Is there a definitive list of features that are not supported here? I'm looking to adapt my eslint config to match.

it's documented here https://github.com/nodejs/node/blob/main/doc/api/typescript.md but you should expect it to change in the future

@jsejcksn
Copy link

There's a Getting Started article on the Node.js website — Node.js with TypeScript (source) — that might benefit from an update including information about this feature (once the time is right). I suppose a linked issue should be created in that repository, but wasn't sure whether it should be included somewhere in the list here, too.

@shawnmcknight
Copy link

  • Enable running TypeScript files in node_modules

Caution

Currently there is no consensus whether Node.js should run TypeScript files inside `node_modules. Currently it is not supported to avoid package maintainers to release TS only package. I'm personally in favor of supporting it.

I understand the argument against encouraging package maintainers from releasing TS only packages. However, one additional thing to consider here is how node operates with internal packages in a monorepo. I'm not familiar with how other package managers work in this regard today, but pnpm will make links for internal workspace protocol packages as node_modules references in the dependent package. If node_modules support is not available, that would still then require a build step for any internal dependencies. I'm not sure how you could differentiate between these internal dependencies versus published packages, but being able to bypass a build step for internal packages would be extremely beneficial for monorepos structured this way.

@jakebailey
Copy link

I'm not sure how you could differentiate between these internal dependencies versus published packages, but being able to bypass a build step for internal packages would be extremely beneficial for monorepos structured this way.

I would think that this detail wouldn't matter because the realpath of these paths would leave node_modules. However, I'm not 100% certain how one would set up a monorepo with TS support via this flag anyway; you'd need to point things like export maps to TS files, making those export maps broken if packages are ever published, unless you go the "live types" route with a monorepo-local custom condition.

@shawnmcknight
Copy link

shawnmcknight commented Jul 26, 2024

I would think that this detail wouldn't matter because the realpath of these paths would leave node_modules

This might just work out of the box then. I haven't tested with the new experimental flag yet, but it would be awesome if it just worked without any other considerations. 👍

However, I'm not 100% certain how one would set up a monorepo with TS support via this flag anyway; you'd need to point things like export maps to TS files, making those export maps broken if packages are ever published, unless you go the "live types" route with a monorepo-local custom condition.

In my use case, I wasn't thinking of publishing because the repo in question isn't published but is our internal backend runtime. With respect to things like export maps, converting them at build-time is actually how we make these internal packages work today. The exports and/or main properties in package.json are referencing the .ts files in the source. This allows for internal type checking to use the dependencies' .ts files and for ts-node to operate against the source file even in the dependencies. At build time, the exports and/or main properties are converted from src/*.ts to dist/*.js so the dependent packages are then using the transpiled JS.

I'm unsure if this is a common use case in the rest of the world, but I'm optimistic based on the realpath comment above that this might just work without any additional overhead.

@Daniel15
Copy link

Is the idea to eventually support other typing systems (for example, Flow, Hegel, or whatever comes next after TypeScript) like https://github.com/tc39/proposal-type-annotations proposes, or is this feature focused solely on TypeScript?

If it's only going to be for TypeScript, wouldn't it be better for the --experimental-strip-types flag to include typescript or ts in its name?

targos pushed a commit to nodejs/node that referenced this issue Jul 28, 2024
PR-URL: #53725
Refs: nodejs/loaders#217
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruy Adorno <ruy@vlt.sh>
RafaelGSS pushed a commit to nodejs/node that referenced this issue Aug 5, 2024
PR-URL: #53725
Refs: nodejs/loaders#217
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruy Adorno <ruy@vlt.sh>
@marco-ippolito marco-ippolito added the typescript-agenda Issue or PRs to be discussed during TypeScript team meeting label Aug 7, 2024
@alshdavid
Copy link

alshdavid commented Aug 13, 2024

My understanding is that the inclusion of TypeScript support in Nodejs is to simplify the development cycle - where distributed code should still be transpiled to JavaScript before consumption (npm, production, etc).

Nodejs-valid TypeScript requires the .ts extension to be supplied in the import specifier. The issue is that there is no easy way to transpile imports with .ts extensions to .js.

The official TypeScript compiler does not support transforming imports due to the difficulty of handling dynamic imports.

SWC has a plugin that can rewrite import extensions, but it skips dynamic imports and is annoying to set up. Ideally, this additional tooling would be avoided as it diminishes the benefit of using built-in TS support over a loader.

Can we get guidance on how to properly transpile Nodejs-valid TypeScript to valid JavaScript for distribution?

@marco-ippolito
Copy link
Member Author

marco-ippolito commented Aug 14, 2024

Some discussion going on here #214.
I think we still need to figure out with @nodejs/typescript
1 - A recomended tsconfig
2 - How to solve the extension resolution during compilation
3 - add to amaro a way to perform the extension resolution wheh used as external loader

@silverwind
Copy link

silverwind commented Aug 14, 2024

Nodejs-valid TypeScript requires the .ts extension to be supplied in the import specifier. The issue is that there is no easy way to transpile imports with .ts extensions to .js.

The extension requirement comes from ESM support in Node which has always required a extension. I think it's a good requirement because it eliminates ambiguity when multiple files exist with the same name and it makes the module resolution faster. Imho people should find some codemod tool to adds the extension to their imports.

@JakobJingleheimer
Copy link
Contributor

Imho people should find some codemod tool to adds the extension to their imports.

I'm working on creating one. I'll post once it's ready. I plan for it to correct bad file extensions (ex .js when the file is actually .ts and there is no such JS file) and when the file extension is altogether missing in the specifier.

@alshdavid
Copy link

The extension requirement comes from ESM support in Node which has always required a extension. I think it's a good requirement because it eliminates ambiguity when multiple files exist with the same name and it makes the module resolution faster. Imho people should find some codemod tool to adds the extension to their imports.

I 100% agree with this take. I think it was a good call to use explicit file paths as it avoids confusion and simplifies resolution.

@JakobJingleheimer
Copy link
Contributor

Codemod is WIP: https://github.com/JakobJingleheimer/correct-ts-specifiers currently struggling with jscodeshift, which seems to be the biggest yet is almost completely undocumented (as is codemod itself 😢).

@Daniel15
Copy link

Daniel15 commented Aug 15, 2024

currently struggling with jscodeshift, which seems to be the biggest yet is almost completely undocumented

@JakobJingleheimer It's being worked on 😄 We (jscodeshift maintainers) launched a new docs site launched recently, but is still a work-in-progress: https://jscodeshift.com/build/api-reference/

Not sure if it's mentioned in the docs, but https://astexplorer.net/ is also a very good resource for inspecting the AST and writing codemods. https://codemod.com/ have Codemod Studio too, but I haven't tried it yet.

@alexbit-codemod
Copy link

alexbit-codemod commented Aug 15, 2024

Codemod is WIP: https://github.com/JakobJingleheimer/correct-ts-specifiers currently struggling with jscodeshift, which seems to be the biggest yet is almost completely undocumented (as is codemod itself 😢).

@JakobJingleheimer, founder of Codemod here (also maintaining jscodeshift, thanks @Daniel15 & @karlhorky for the mention)
would love to connect and help with any codemod questions, tooling, or documentation you need. feel free to ping me on X or in our community: https://go.codemod.com/community

@mitschabaude
Copy link

mitschabaude commented Aug 19, 2024

Nodejs-valid TypeScript requires the .ts extension to be supplied in the import specifier. The issue is that there is no easy way to transpile imports with .ts extensions to .js.

The extension requirement comes from ESM support in Node which has always required a extension. I think it's a good requirement because it eliminates ambiguity when multiple files exist with the same name and it makes the module resolution faster. Imho people should find some codemod tool to adds the extension to their imports.

@silverwind the problem isn't that the strip-types MVP requires an extension (that makes perfect sense), but specifically that it requires the .ts extension while tsc requires the .js extension for a TS file. So, as it stands strip-types is incompatible with tsc in an annoying way.

This could be resolved by using a codemod transpilation tool either before running tsc or node, but that almost makes strip-types useless, as the whole point AFAIU was to make TS transpilation built into Nodejs, which it isn't if we need yet another tool to make it work. (See more discussion here)

It could also be resolved on the tsc side, by rewriting .ts imports, which would be even nicer (.ts imports are more natural for TS files). But then again, tsc is the established tool and strip-types is the experimental newcomer, so I think there's an argument that strip-types should ensure compatibility anyway.

@JakobJingleheimer
Copy link
Contributor

JakobJingleheimer commented Aug 19, 2024

So, as it stands strip-types is incompatible with tsc in an annoying way.

Maybe splitting hairs a bit, but it's the other way around: tsc is incompatible with standards in an annoying way.

The codemod is almost ready: https://github.com/JakobJingleheimer/correct-ts-specifiers

@marco-ippolito is this codemod something we might want to make official? (Eg putting it under the @nodejs or @pkg namespace in npm)

that almost makes strip-types useless

Definitely not. The codemod is a one-and-done step to fix issues in source-code, very similar to a migration; thereafter, just write the source-code properly and no extra tools are required.

there's an argument that strip-types should ensure compatibility anyway

Being first and wrong is still being wrong. That's not an argument in favour of changing our implementation (valid new arguments are welcome of course 🙂).

@marco-ippolito
Copy link
Member Author

@JakobJingleheimer we can add it to amaro and figure it out

@mitschabaude
Copy link

mitschabaude commented Aug 19, 2024

The codemod is a one-and-done step to fix issues in source-code, very similar to a migration; thereafter, just write the source-code properly and no extra tools are required.

@JakobJingleheimer there's a misunderstanding here. I agree it's no big deal to migrate an existing codebase from .js to .ts, I'd probably do that with find-and-replace.

However, that's not the issue that I was trying to bring up. The issue is in code bases that want to continue using tsc to compile their TS to JS, and therefore use .js. strip-types is incompatible with that flow.

(In case you wondered: no, I don't always want to use a third-party tool instead of tsc to transpile the TS source, to get .ts imports to work; tsc has more compatibility with tsconfig options and various TS versions than any other tool; also, what's the point of strip-types if I need a third-party transpiler in my toolchain anyway)

@mitschabaude
Copy link

mitschabaude commented Aug 19, 2024

Being first and wrong is still being wrong. That's not an argument in favour of changing our implementation (valid new arguments are welcome of course 🙂).

Ok! Then my argument is that changing your implementation would make strip-types more useful, and therefore more widely adopted.

@marco-ippolito
Copy link
Member Author

marco-ippolito commented Aug 19, 2024

@mitschabaude its clear there is incompatibility between tools.
From my point of view Node should not support guessing the extension.
I believe typescript is working to add a flag to emit import with '.ts' extension.
Also, this is an experimental feature, at an early stage, so its fair that is not fully compatible with existing tooling 🙂

@mitschabaude
Copy link

Also, this is an experimental feature, at an early stage, so its fair that is not fully compatible with existing tooling 🙂

Definitely and I didn't want to suggest that it should. I think the direction taken with strip-types is great, congrats on the work so far 🙌🏻

I'm really just talking about the roadmap for this feature. Currently the roadmap has the issue listed as "wontfix". I'd love for that to change. I'm also willing to help make strip-types work with js imports. My intuition is that this should be possible at no additional cost to currently supported use cases.

@JakobJingleheimer
Copy link
Contributor

JakobJingleheimer commented Aug 19, 2024

I don't always want to use a third-party tool instead of tsc to transpile the TS source, to get .ts imports to work; tsc has more compatibility with tsconfig options and various TS versions than any other tool

Is there a specific and compelling tsconfig option that only tsc supports? If so, that could be persuasive. I've used esbuild as my transpiler for quite some time though, and I've never encountered anything it couldn't handle.

Since everyone seems to agree TS's behaviour is wrong, I'm still inclined to say regardless that that would be TypeScript's problem, and you're asking us instead of them to change something because we actually consider feedback 🙃 (which is a little funny as I've so far basically been saying "no" 😅)

Buuuut as Marco pointed out, this may be moot if tsc is soon to handle proper file extensions.

I'm also willing to help make strip-types work with js imports.

Very appreciate that you're willing to contribute—contributions generally super welcome! However, there is significant opposition to this particular item within node, not to mention it would violate spec; barring a very unexpected persuasive argument (I'm not so arrogant to say I'm right no matter what 😉), I would block this. It is already possible and very easy to do this within node with tools that already exist: a resolve hook. I even wrote such a hook that will do what you want.


Should this be moved to a separate issue/discussion? I think this is getting a bit too specific for this general roadmap issue.

@allisonkarlitskaya
Copy link

I love absolutely everything that's going on here, so thank you to everyone who has been involved in it. I'm particularly impressed to see the communication and coordination going on between the node and TypeScript teams.

Here's a question from the peanut gallery: --experimental-strip-types has been released as part of v22.6.0. Will --experimental-transform-types land in some v22 release soon as well? It would be lovely to see this packaged in Fedora so that we can start playing around with it in earnest.

@silverwind
Copy link

I love absolutely everything that's going on here, so thank you to everyone who has been involved in it. I'm particularly impressed to see the communication and coordination going on between the node and TypeScript teams.

Here's a question from the peanut gallery: --experimental-strip-types has been released as part of v22.6.0. Will --experimental-transform-types land in some v22 release soon as well? It would be lovely to see this packaged in Fedora so that we can start playing around with it in earnest.

As I understand it, everything that's on main branch until the v22 LTS is forked on 2024-10-29 will be included in a upcoming v22 release.

@mitschabaude
Copy link

Should this be moved to a separate issue/discussion? I think this is getting a bit too specific for this general roadmap issue.

Agree, answered here: #214 (comment)

@marco-ippolito
Copy link
Member Author

@allisonkarlitskaya yes, --experimental-transform-types is going to be released in v22.7.0 which is scheduled for today/tomorrow

@damianobarbati
Copy link

@marco-ippolito can't find proper info on the following, please let me know if I'm repeating a known issue:

  1. How will it behave with dynamic imports? Will the flag be applied also to dynamically imported modules?
  2. How will it deal with node_modules? Will it strips recursively?

I guess authors will have to compile/transpile the code because recursively stripping the imported files could result in breakage due to enums/namespace/decorators. Correct?

@marco-ippolito
Copy link
Member Author

marco-ippolito commented Sep 12, 2024

@marco-ippolito can't find proper info on the following, please let me know if I'm repeating a known issue:

  1. How will it behave with dynamic imports? Will the flag be applied also to dynamically imported modules?
  2. How will it deal with node_modules? Will it strips recursively?

I guess authors will have to compile/transpile the code because recursively stripping the imported files could result in breakage due to enums/namespace/decorators. Correct?

please refer to the documentation https://nodejs.org/docs/latest/api/typescript.html#type-stripping
To answer briefly:

  • dynamic imports work as expected when they have .ts extension
  • execution inside node_modules is not supported

If needed there is the flag --experimental-transform-types that supports enums, namespaces etc...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
typescript Discussions related to TypeScript typescript-agenda Issue or PRs to be discussed during TypeScript team meeting
Projects
None yet
Development

No branches or pull requests