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

Support typescript with --experimental-strip-types #208

Closed
2 of 8 tasks
marco-ippolito opened this issue Jul 2, 2024 · 89 comments
Closed
2 of 8 tasks

Support typescript with --experimental-strip-types #208

marco-ippolito opened this issue Jul 2, 2024 · 89 comments
Labels
typescript Discussions related to TypeScript

Comments

@marco-ippolito
Copy link
Member

marco-ippolito commented Jul 2, 2024

Add this issue to keep track of the work for supporting typescript out of the box, in the PR you can find documentation etc...
The main idea is to use type-stripping.
This is achieved ideally through the use of an external dependency, in my opinion @swc/wasm-typescript.
I would like to have something basic and usable to propose to the public.

Tracking points to discuss:

  • Support commonjs syntax
  • SourceMaps (enabling them with a flag)
  • Type stripping on .cts
  • Add a flag to enable transformation to support TS only features (enums etc...)
  • Extension guessing (.js because of transpilation => but at runtime is a TS)
  • Transpiling .ts files in the node_modules
  • TypeScript linting
  • Externalize swc wasm

Roadmap: #217

@mcollina
Copy link
Member

mcollina commented Jul 2, 2024

I have two concerns (none of them blocking):

  1. Is swc on board with following our LTS? This seems a massive dependency we should not take on lightly.
  2. TypeScript does not follow semver, and the language evolves relatively quickly. Is this approach stable long term? Everybody seems always to be supporting the latest version of TypeScript.

My proposal in nodejs/node#43818 was to have a strategy for doing this automatically but not vendor anything. Something along the lines of:

$ node script.ts
Typescript support is missing, install it with:
npm i --location=global typescript-node-core
$ npm i --location=global typescript-node-core
...
$ node script.ts
"Hello World"

An alternative (possibly better for long-term support):

$ node script.ts
Typescript support is missing, install it with:
npx ts-node-core

Learn about TypeScript support at: ...
$ npx ts-node-core
Detected Node.js v24.42.0
What typescript support would you like?
[ ] type stripping - swc@22
[ ] type checking - tsc@23

...Installing

$ node script.ts
"Hello World"

@marco-ippolito
Copy link
Member Author

marco-ippolito commented Jul 2, 2024

To answer to your points:

  1. SWC seems to be a mature project, it is already used by Deno for our same purpose, has Apache-2 license which is fine.
    I'm using @swc/wasm, the wasm version to avoid compiling rust. We probably should ask the blessing from the maintainer @kdy1 and maybe release a version that we dont have to patch 😄
  2. With typestripping we dont really care what version of typescript the user is using because we just remove it (unless there is some syntax not supported by SWC), we dont perform type checking at all. I think SWC moves quickly enough.
    Also asking the user to install a dependency is imho a suboptimal DX, users just want to be able to run node foo.ts

If this proposal is accepted we could also add hooks to customizate the transpilation, and many more things that come out of the box from swc

@kdy1
Copy link
Member

kdy1 commented Jul 2, 2024

I can configure a publish pipeline for a separate package/binary, or a Wasm publish pipeline if node.js will use SWC for this task.

@marco-ippolito
Copy link
Member Author

marco-ippolito commented Jul 2, 2024

I can configure a publish pipeline for a separate package/binary, or a Wasm publish pipeline if node.js will use SWC for this task.

If we are able to ship the initial PR with v1.6.6 for the next updates that would be amazing, thank you

@benjamingr
Copy link
Member

+1 for all the suggestions and also +1 for @kdy1 have nothing but nice things to say about him and swc + there is prior art in other runtimes.

@GeoffreyBooth
Copy link
Member

If what we ship is type stripping, then we’re essentially shipping experimental support for the Type Annotations proposal. Which is good! I feel like that’s what does make sense to include in Node core, and I hope that proposal graduates and V8 adds support and then we can drop our support for doing it ourselves.

In that vein, maybe a flag of --experimental-strip-types might make more sense? To clarify to users that our intended scope is not to support all of TypeScript’s syntax or features, such as paths or enums or type checking; we just strip types and that’s it, and if they want more they can register module customization hooks from a userland library. And type stripping should be stable and unlikely to change as TypeScript evolves.

@marco-ippolito
Copy link
Member Author

marco-ippolito commented Jul 2, 2024

If what we ship is type stripping, then we’re essentially shipping experimental support for the Type Annotations proposal. Which is good! I feel like that’s what does make sense to include in Node core, and I hope that proposal graduates and V8 adds support and then we can drop our support for doing it ourselves.

In that vein, maybe a flag of --experimental-strip-types might make more sense? To clarify to users that our intended scope is not to support all of TypeScript’s syntax or features, such as paths or enums or type checking; we just strip types and that’s it, and if they want more they can register module customization hooks from a userland library. And type stripping should be stable and unlikely to change as TypeScript evolves.

Not against renaming the flag, but for this very limited features that we are adding, --experimental-typescript makes more sense because we are actually only supporting .ts files, (no tsx or other flavors), but once this lands and remove the flag, we will think on how to make it customizable, and increase the number of flavor, that would be a real type-stripping

@GeoffreyBooth
Copy link
Member

This was discussed today in the loaders meeting:

  • I think there was general agreement that type stripping (but probably not more) is something that we would like to see in core, either soon or eventually.
  • @joyeecheung asked that @marco-ippolito’s branch be implemented as a package similar to Undici that can be developed outside of the core release cycle and eventually merged in when it’s mature.
  • To exist in core, support would need to both take advantage of being in core, such as being native; and avoid implementing features of TypeScript that may change faster than our release cycle (such as type checking, and any transforms such as enums, decorators, ESM-CJS transpilation and others).
  • @legendecas will schedule a meeting with @marco-ippolito and some champions of the Type Annotations proposal to discuss collaboration and next steps toward potentially implementing that proposal in Node core.

@legendecas legendecas added loaders-agenda Issues and PRs to discuss during the meetings of the Loaders team and removed loaders-agenda Issues and PRs to discuss during the meetings of the Loaders team labels Jul 2, 2024
@kdy1
Copy link
Member

kdy1 commented Jul 3, 2024

I created https://www.npmjs.com/package/@swc/wasm-typescript, which barely strips out the type.

And I wrote documentation for it at https://swc.rs/docs/references/wasm-typescript

@GeoffreyBooth
Copy link
Member

which barely strips out the type.

How can this be used to just strip types and nothing more?

@kdy1
Copy link
Member

kdy1 commented Jul 3, 2024

@GeoffreyBooth
Copy link
Member

See swc.rs/docs/references/wasm-typescript

I saw that. What settings would I use to just strip types and nothing more? No support for enums, decorators, transpiling module import/export, etc. As in, if I wanted to use this to implement the Type Annotations proposal and error on any syntax that requires transforms.

@kdy1
Copy link
Member

kdy1 commented Jul 3, 2024

It's not transform() of @swc/core. It's a completely different thing.

It

  • strips type
  • supports enum
  • does not support transpiling decorator (parsing/codegen is possible with a config)
  • import/exports are not touched, except type-only imports. (Including imports not used as values, per config)

These operations is one typescript set, but I can remove the enum part and/or handling code for type-only imports.
I'll add options for them soon.

@GeoffreyBooth
Copy link
Member

These operations is one typescript set, but I can remove the enum part and/or handling code for type-only imports.
I’ll add options for them soon.

What I would like is for some option or collection of options where I can use SWC to implement the Type Annotations proposal, but nothing more. In particular, see https://github.com/tc39/proposal-type-annotations#intentional-omissions:

Intentional Omissions

We consider the following items explicitly excluded from the scope of this proposal.

Omitted: TypeScript-specific features that generate code

Some constructs in TypeScript are not supported by this proposal because they have runtime semantics, generating JavaScript code rather than simply being stripped out and ignored.
These constructs are not in the scope of this proposal, but could be added by separate TC39 proposals.

Omitted: JSX

JSX is an XML-like syntax extension to JavaScript that is designed to be transformed by a pre-processor into valid JavaScript.
It is widely used in the React ecosystem, but it has been used for different libraries and frameworks.
Because JSX directly interleaves with JavaScript code, compilers and type-checkers for JavaScript typically also support checking and transforming JSX as well.
Some users may hope that the JSX transform could also be directly supported by ECMAScript, to expand the set of use-cases that can be handled without a build step.

We do not consider JSX to be in scope of this proposal because:

  • JSX is an orthogonal feature that is independent of optional static types. This proposal does not affect the viability of introducing JSX into ECMAScript via an independent proposal.
  • JSX syntax expands into meaningful JavaScript code when transformed. This proposal is only concerned with syntax erasure.

This section basically defines the parts of TypeScript that can’t be stripped: these are transforms, where SWC isn’t merely stripping these things out but rather injecting new JavaScript in their place. If our goal is just to implement type stripping, then we should throw exceptions when encountering any TypeScript syntax that requires transformation.

To be even more careful, we could also throw on the various items in https://github.com/tc39/proposal-type-annotations#up-for-debate, as these are questionable as to whether they can be safely stripped: stuff like the class private keyword. I think it’s better to be conservative and error on all of these if possible, and potentially add things back one-by-one if a particular syntax can be shown to be strippable safely.

The vast majority of TypeScript exists outside of these exceptions; most projects should be able to get just about all the benefits of TypeScript even without this handful of exclusions. The result of adding the ability to run Type Annotations Proposal-compliant TypeScript will be to encourage people to write TypeScript code that is compatible with a potential future where Type Annotations graduates to be part of the language itself. That in itself is a win for the entire ecosystem, as we begin the process of folding TypeScript into the spec and bringing the users along with it. And for Node in particular, once Type Annotations hopefully becomes part of the spec, V8 will implement it and then we can rely on that rather than maintaining this ourselves.

kdy1 added a commit to swc-project/swc that referenced this issue Jul 3, 2024
**Description:**

The Node.js team wants a Wasm package that only implements type annotation proposals. So, we need to refactor our TypeScript transform to allow it.


**Related issue:**

 - nodejs/loaders#208 (comment)
@JakobJingleheimer
Copy link
Contributor

@GeoffreyBooth I get why you're suggesting erroring on things that result in code being generated. But as a TypeScript user, I have enums etc. and if this errors when running on my completely valid code, it's not fit for purpose. I would not change my code to support an incomplete feature—I would just use something else. I imagine that will be the case for many if not all effected users. And enums are not rarely used, so I think such a decision here is a square wheel.

@aduh95
Copy link
Contributor

aduh95 commented Jul 3, 2024

@JakobJingleheimer I think the main target is new users / new projects, I don't think it's realistic (or useful) to try to support existing project – if you already have setup your tooling, why would you move to the experimental support in Node.js, when you can keep using the tooling that works? On the other hand, if you're starting, not having to setup tooling is huge, and if the tradeoff is to not use enums, that's really a no-brainer IMO.

@marco-ippolito
Copy link
Member Author

marco-ippolito commented Jul 3, 2024

This was discussed today in the loaders meeting:

  • I think there was general agreement that type stripping (but probably not more) is something that we would like to see in core, either soon or eventually.
  • @joyeecheung asked that @marco-ippolito’s branch be implemented as a package similar to Undici that can be developed outside of the core release cycle and eventually merged in when it’s mature.
  • To exist in core, support would need to both take advantage of being in core, such as being native; and avoid implementing features of TypeScript that may change faster than our release cycle (such as type checking, and any transforms such as enums, decorators, ESM-CJS transpilation and others).
  • @legendecas will schedule a meeting with @marco-ippolito and some champions of the Type Annotations proposal to discuss collaboration and next steps toward potentially implementing that proposal in Node core.

My view on this

  • I totally agree, lets just support type stripping
  • I don't see this implemented outside of core simply because there are plenty of alternatives, what we want to improve here is the DX. If this lands even as an incomplete feature, I'm very positive community will be supportive and will see a massive improvement. This imho just needs someone to do the heavy lifting of the first implementation.
  • I don't agree that this has to land as NATIVE but the goal is to move it to native side.
  • I will be happy to sync with Type Annotations proposal but I'm not gonna let this bogged down by a Stage 1 Proposal

I'm happy ok with renaming it --experimental-strip-types, IF we support real type stripping.
Changed my mind, actually lets remove enums, maybe it will drive the community not to use them or be implemented in the language 😃

@nicolo-ribaudo
Copy link

Note that the type annotations proposal has different syntax from TypeScript, so I recommend not conflating the two even if your goal is to just strip TS types.

@JakobJingleheimer
Copy link
Contributor

not having to setup tooling is huge

Setting up the tooling for this is trivial:

npm i nodejs-loaders

node --loader=nodejs-loaders/dev/tsx

Bam. Done.

@marco-ippolito
Copy link
Member Author

not having to setup tooling is huge

Setting up the tooling for this is trivial:

npm i nodejs-loaders

node --loader=nodejs-loaders/dev/tsx

Bam. Done.

This is exactly what users dont want to do 😆 and we have to acknowledge that node foo.ts is what everybody wants

@JakobJingleheimer
Copy link
Contributor

JakobJingleheimer commented Jul 3, 2024

we have to acknowledge that node foo.ts is what everybody wants

Mm, I get that.

What about some middle-ground: I expect users regardless of project pre-existing or not will not accept (read: ridicule) the missing features we're discussing disallowing. SWC can already handle those (if I'm not mistaken).

Proposed compromise: an additional flag to enable them, like --experimental-generative-transpilation that enables enums, decorators, etc already supported.

@marco-ippolito
Copy link
Member Author

marco-ippolito commented Jul 3, 2024

currently my pr already supports all of that with the version of swc we are using. BUT it's not real type-stripping.
--experimental-generative-transpilation is reasonable

@GeoffreyBooth
Copy link
Member

GeoffreyBooth commented Jul 3, 2024

Proposed compromise: an additional flag to enable them, like --experimental-generative-transpilation that enables enums, decorators, etc already supported.

We could potentially do this, or we could push such users into importing a library. I don't see us being able to ever unflag transforms because of the semver problem and TypeScript moving faster than we do and not being specified. For that reason transforms feel like a bad fit for core.

@marco-ippolito
Copy link
Member Author

Proposed compromise: an additional flag to enable them, like --experimental-generative-transpilation that enables enums, decorators, etc already supported.

We could potentially do this, or we could push such users into importing a library. I don't see us being able to ever unflag transforms because of the semver problem and TypeScript moving faster than we do and not being specified. For that reason transforms feel like a bad fit for core.

We can give it a try and see how it goes

@legendecas
Copy link
Member

legendecas commented Jul 3, 2024

It would be better to have TypeScript team buy-in to build in TypeScript syntax, particularly the syntax and transformation would be bound to Node.js release cycles. Ultimately, this type-stripping-only support in Node.js still requires TypeScript to perform type-checking to make it a complete DX. /cc @DanielRosenwasser @robpalme

Additionally, I didn't find how ESM/CJS support would work in the draft PR (e.g. it doesn't support transpiling CJS at the moment). I'd like to learn more about the support plan since TypeScript has Node.js targetted options like module: nodenext and there are open issues like nodejs/node#50981.

@GeoffreyBooth
Copy link
Member

GeoffreyBooth commented Jul 13, 2024

@DanielRosenwasser Thanks to you and your team for joining today’s call. My takeaways from the call are:

  • The TypeScript team prefers that users get the “full” TypeScript experience already provided by --import=tsx or similar, and approves of UX improvements to make it easier to set up customization hooks. Node will continue supporting this method, and there is work ongoing in this area.

  • The TypeScript team has no particular objections to --experimental-strip-types, other than a general concern that users will be frustrated by the feature and expect it to be able to do more than it’s designed to do. We understand this UX concern and for the moment I think we’re willing to take the chance that users will find the increased performance worth the tradeoff of unsupported features, but the feature is experimental and we can always change course if the feedback proves otherwise.

  • The TypeScript team says that TypeScript’s features that require JavaScript generation are few and far between, are unlikely to ever change, and it is unlikely that TypeScript will add more of them. This means that we could support transforms without a semver concern, either as part of a renamed --experimental-strip-types or as part of a separate flag; but we would need to add support for source maps in order to support transforms. Transforms would therefore incur a noticeable performance hit, and so I think for now we want to see how users react to a more-optimized transform-less type stripping feature (with transforms still available via the --import=tsx approach).

  • The TypeScript team is worried about package authors publishing untranspiled TypeScript files to the npm registry. Others on the call consider this a ship that has already sailed, and doing things on our side to try to discourage the practice (such as trying to prevent TypeScript support in node_modules) are likely to have unwanted side effects such as breaking monorepo workflows.

Does this match your perception? Am I missing anything?

If there are no specific notes around --experimental-strip-types, can we feel free to proceed with that PR or would you like more time to review it further?

@arcanis
Copy link
Contributor

arcanis commented Jul 13, 2024

I think we’re willing to take the chance that users will find the increased performance worth the tradeoff of unsupported features

I still find the supporting research lacking. Given that the run-TS-transparently ecosystem already settled a long time ago on "run the TS transforms", I would expect the default implementation to be matching behaviour, not something new and confusing ("I'm following a TS tutorial but enums don't work, why is that?").

I also have more abstract concerns in the way this feature is being developed; it feels like we're kinda hijacking the development of a third-party project by making decisions about what "TypeScript support" means in their place (enums, but generally speaking by, pick-and-choosing language features). Yes, the flag you envision is --experimental-strip-types, but the loader would be tied to .ts extensions - it doesn't feel right to me that Node would ship an arbitrary subset and pretend it's TS.

The TypeScript team is worried about package authors publishing untranspiled TypeScript files to the npm registry. Others on the call consider this a ship that has already sailed.

It didn't sail since --experimental-strip-types hasn't sailed. If you talk about TS code already being published (I'm not aware of any amongst popular tools?), then I'd be curious to see a short list - if only to see if they use enums or not, and thus whether they'd be compatible with your approach.

@mcollina
Copy link
Member

Node.js should only strip types and execute javascript rather then support all of ts caveats and configuration that can be done by user space tools.

I agree. This means that whatever subset of ts we support natively has to be compilable by tsc. This means that we should support importing “.js” files where a “.ts” file is present on disk.

@rauschma
Copy link

rauschma commented Jul 13, 2024

This is subjective but:

  • I find Deno’s approach most straightforward and least confusing: .ts means TypeScript, .js means JavaScript.
    • There are many questions online w.r.t. why .ts doesn’t work in imports.
    • Deno’s approach sets a precedent for non-transpiled code.
  • I would switch to that approach in my Node.js code bases (written in TypeScript, transpiled to JavaScript) if I could.
  • If web browsers are to maybe support this in the long run, then I don’t see how going from .js to .ts (if the former doesn’t exist) would work there.

@jakebailey
Copy link

There are a few topics that I don't think we had time to address on the call (due to talking about other things; if we meet again it would be good to time box or have a list of specific topics ahead of time).


It seems like what users "want" (per comments before in this thread) is to be able to use the latest version of TypeScript. If the TS->JS code lives within Node.js, is there a method by which someone can upgrade that transform out of band? Otherwise, it seems like the only option is to go back to using loaders, or to eject and go back to transpiling on-disk.


I'll also note that I don't personally find arguments relating to the "type annotations" proposal to be convincing.

If the Type Annotations proposal graduates, then it will be implemented in V8 and a future version of Node will get type stripping for free. Then we can remove SWC from our pipeline [...]

This may not actually turn out to be true, depending on the way the proposal goes. At one point, the proposal worked like a "token soup" that allowed a wider range of freedom for future language changes, but the current spec encodes specific syntax that would be allowed. This means that if TypeScript ever adds anything new that doesn't fit that spec (e.g. satisfies, deferred parameter modifiers in 5.6), one can't simply treat all TS files as "JS with annotations".

Supporting TS with a type-stripper doen't seem like a gateway to "JS with annotations" in that way. By nature, vendoring SWC is just capturing a point-in-time TypeScript syntax, and would too continue to evolve.


As for TS extensions, theoretically that should all "work" via allowImportingTsExtensions, but it's definitely going to be a hazard to expect users to publish. You definitely do not want to have TypeScript load someone else's TypeScript code unless you're absolutely certain they are also using the same compiler options you are. We've already talked about the emit differences that can happen (class fields, decorators; cases where TS adopted something before it was standardizes), but what's also problematic from a user experience perspective is when modules resolve to someone else's TS code, leading to:

  • Loss of performance due to rechecking of someone else's code.
  • Potential errors due to differing strictness / beahvior compiler options. For example, exactOptionalPropertyTypes, noUncheckedIndexAccess, noImplicitAny.
  • Errors due to someone using newer TS than the consuming code, where emit would have prevented that (or via a tool like downlevel-dts for compatibility).
    • TypeScript included versioned syntax for package.json export mapping for types (along with typesVersions) to mitigate this, but no such thing would exist for executable code.

JSR and Deno were brought up in this context, but they are "special" in that for non-Deno users, JSR serves up .js and .d.ts files, avoiding the above, while Deno is strict about which options can even be set by nature of having a completely different config scheme.

The TypeScript team is worried about package authors publishing untranspiled TypeScript files to the npm registry. Others on the call consider this a ship that has already sailed, and doing things on our side to try to discourage the practice (such as trying to prevent TypeScript support in node_modules) are likely to have unwanted side effects such as breaking monorepo workflows.

I don't personally think the "ship has already sailed" on this front, solely from the perspecive that module resolution of existing packages could not have mapped to .ts files and actually been able to run. esbuild in particular takes care to not load .ts files in node_modules due to the previously mentioned gotchas, though by "only stripping", the emit differences do go away (leaving only the other two gotchas).

@GeoffreyBooth
Copy link
Member

It seems like what users “want” (per comments before in this thread) is to be able to use the latest version of TypeScript.

This is already possible today: --import=tsx.

If the TS->JS code lives within Node.js, is there a method by which someone can upgrade that transform out of band?

The type stripping we’re proposing won’t do any transforming, just replacing types with whitespace (which maybe is what you’re considering a transform). No this would not be upgradable. We’ll update SWC with future versions of Node like any other dependency, and new TypeScript syntaxes that need to get added to SWC for stripping can be added in future versions of SWC and then into Node as a semver-minor update. Older versions of Node and SWC won’t be able to strip those new syntaxes and would error on them, just like old versions of Node and V8 error on new JavaScript syntaxes.

Otherwise, it seems like the only option is to go back to using loaders

The module customization hooks aren’t going away and are intended for the users who want “full” TypeScript, including being able to run the latest TypeScript version and configure it via tsconfig.json. Type stripping is an alternative, not a replacement.

You definitely do not want to have TypeScript load someone else’s TypeScript code unless you’re absolutely certain they are also using the same compiler options you are.

Yes, but this is already a hazard today and isn’t really affected by the type stripping proposal. I understand the TypeScript team’s desire to avoid the problems associated with publishing TypeScript to the npm registry, but I don’t think the type stripping proposal really makes things any worse than they already are; or what we could put in Node that would improve the situation without breaking valid use cases such as monorepos or users consuming their own local packages written in TypeScript.

Basically I’m not seeing any concerns with type stripping per se, just a general “we think users prefer what tsc or tsx have to offer.” Which is fine; but those tools already exist and will continue to exist, and the users who prefer them can continue to use them, so I’m not sure how relevant this feedback is to the type stripping proposal. If users don’t value the type stripping feature then they won’t use it, and everyone moves on, and that’s fine.

@JakobJingleheimer
Copy link
Contributor

This means that we should support importing “.js” files where a “.ts” file is present on disk.

I vehemently disagree and I will block this to the utmost of my ability.

@JakobJingleheimer
Copy link
Contributor

JakobJingleheimer commented Jul 13, 2024

I like what Geoffrey is suggesting: if you want basic "node can read ts code", use this feature. If you want to get fancy, that takes a tinsy bit of effort.

Regarding old code bases being incompatible with the stripping feature because they containing erroneous file extensions within import specifiers, that seems a very real concern. I was just having this discussion in my own loaders lib, and I'm thinking to create a codemod (as a separate lib) to correct invalid .js.ts (and other flavours like .jsx.tsx). This would work for both this node feature and projects using tsc for type-checking (by leveraging the aforementioned tsconfig option). It is fully compatible with built-in features of VS Code as well.

@GeoffreyBooth GeoffreyBooth added the typescript Discussions related to TypeScript label Jul 13, 2024
@jakebailey
Copy link

The type stripping we’re proposing won’t do any transforming, just replacing types with whitespace (which maybe is what you’re considering a transform).

Yes, in my mind everything is just transforms. 😄

(This discussion was the first I had head the term "type stripping"; in tsc, the same pass also handles all other TS special syntax.)

Basically I’m not seeing any concerns with type stripping per se, just a general “we think users prefer what tsc or tsx have to offer.” Which is fine; but those tools already exist and will continue to exist, and the users who prefer them can continue to use them, so I’m not sure how relevant this feedback is to the type stripping proposal.

I think the reason it's getting brought up from different angles is just due to this being motivated by
"TypeScript support" being on top of the survey. Type stripping only, .ts extensions, vendor only, the potential for loading .ts from node_modules, are all certainly the most expedient way to implement things, but I'm still generally worried that this may not match the user expectation of what "TypeScript support" "means".

@GeoffreyBooth
Copy link
Member

I still find the supporting research lacking.

@arcanis I wanted to see how hard it would be to migrate an existing app to use --experimental-strip-types, so I migrated Corepack: https://github.com/nodejs/corepack/compare/main…GeoffreyBooth:corepack:strip-types

I got it as far as being able to print the help text when run with strip-types. To see for yourself, follow these steps:

# Check out the `--experimental-strip-types` branch and build it
git clone -b feat/typescript-support git@github.com:marco-ippolito/node.git
cd node
./configure
make
cd ..

# Check out the migrated Corepack
git clone -b strip-types git@github.com:GeoffreyBooth/corepack.git
cd corepack
npm install # Node wants a real node_modules folder; I didn't figure out Yarn PnP for this test
../node/out/Release/node --experimental-strip-types ./sources/_cli.ts # Prints the help text

Here’s the summary of what I changed:

  • I added file extensions to the import specifiers and added "allowImportingTsExtensions": true to tsconfig.json.
  • I added "type": "module" to the package.json and updated tsconfig.json to use "module": "ESNext".
  • I added with { type: 'json' } to the JSON import statements.
  • I split up the import statements that were importing both types and references into separate import statements, where one was import and the other was import type.
  • I rewrote the public class constructor to use the standard syntax.
  • I rewrote the one enum into an as const object.

The public class constructor update involved changing this:

export class Engine {
  constructor(public config: Config = defaultConfig as Config) {

to this:

export class Engine {
  config: Config;

  constructor(config: Config = defaultConfig as Config) {
    this.config = config;

The enum update involved changing this:

export enum SupportedPackageManagers {
  Npm = `npm`,
  Pnpm = `pnpm`,
  Yarn = `yarn`,
}

export const SupportedPackageManagerSet = new Set<SupportedPackageManagers>(
  Object.values(SupportedPackageManagers),
);

export const SupportedPackageManagerSetWithoutNpm = new Set<SupportedPackageManagers>(
  Object.values(SupportedPackageManagers),
);

// npm is distributed with Node as a builtin; we don't want Corepack to override it unless the npm team is on board
SupportedPackageManagerSetWithoutNpm.delete(SupportedPackageManagers.Npm);

to this:

export const SupportedPackageManagersEnum = {
  Npm: `npm`,
  Pnpm: `pnpm`,
  Yarn: `yarn`,
} as const;

export type SupportedPackageManagers = typeof SupportedPackageManagersEnum[keyof typeof SupportedPackageManagersEnum]
export type SupportedPackageManagersWithoutNpm = Exclude<SupportedPackageManagers, `npm`>;

export const SupportedPackageManagerSet = new Set<SupportedPackageManagers>(
  Object.values(SupportedPackageManagersEnum),
);

// npm is distributed with Node as a builtin; we don't want Corepack to override it unless the npm team is on board
export const SupportedPackageManagerSetWithoutNpm = new Set<SupportedPackageManagersWithoutNpm>(
  Object.values(SupportedPackageManagersEnum).filter((name) => name !== SupportedPackageManagersEnum.Npm),
);

This enum update went a little farther than was minimally necessary, because it bothered me that SupportedPackageManagerSetWithoutNpm was the same type as SupportedPackageManagerSet. You might want to consider making this change on its own merits, separate from any strip-types migration.

Anyway the bottom line is that this wasn’t all that hard, and a bit monotonous; yes @JakobJingleheimer we really need a codemod to add file extensions to import specifiers, and with { type: 'json' } to JSON imports. We also need to write a guide page to teach users how to do this themselves, including techniques for migrating particular syntaxes like enums.

I mentioned in nodejs/node#53725 (comment) that we should hold off on releasing --experimental-strip-types until we can improve the error messages and the docs/guides, and I feel much more strongly about that now. Some of the errors were borderline acceptable—TypeScript parameter property is not supported in strip-only mode—but RuntimeError: unreachable at wasm://wasm/0072633a:wasm-function[1259]:0x1a2e79 for all the missing file extensions is just ridiculous. Maybe we don’t need to fix all of these in the first PR, but we need these to be improved dramatically before we release this.

I also did a quick benchmark using the same node binary to compare --experimental-strip-types against tsx:

hyperfine --warmup 10 --runs 30 \
  '~/Sites/node/node --experimental-strip-types ./sources/_cli.ts' \
  '~/Sites/node/node --import=tsx ./sources/_cli.ts'
Benchmark 1: ~/Sites/node/node --experimental-strip-types ./sources/_cli.ts
  Time (mean ± σ):     152.8 ms ±   3.6 ms    [User: 321.8 ms, System: 29.2 ms]
  Range (min … max):   145.3 ms … 161.2 ms    30 runs

Benchmark 2: ~/Sites/node/node --import=tsx ./sources/_cli.ts
  Time (mean ± σ):     186.4 ms ±   2.6 ms    [User: 208.6 ms, System: 35.0 ms]
  Range (min … max):   179.3 ms … 191.7 ms    30 runs

Summary
  ~/Sites/node/node --experimental-strip-types ./sources/_cli.ts ran
    1.22 ± 0.03 times faster than ~/Sites/node/node --import=tsx ./sources/_cli.ts

@yyx990803
Copy link

yyx990803 commented Jul 14, 2024

@GeoffreyBooth I think you are considering the migration "not that hard" because you are evaluating this from the perspective of an expert who has deep knowledge of Node.js, TypeScript AND this PR's particular implementation. I want to reiterate this is definitely not easy for an average user who just want to "run some TypeScript with Node.js", and catering to such more general users seems to be the original goal of this effort.

A common trap in tooling / framework design is that implementors often underestimate how much these "little inconsistencies" combined together can result in terrible UX.


Another topic I want to bring up is that it seems a major motivation of going with strip-type-only is the assumed performance advantage (and the ability to avoid source maps).

However, I think it's possible to implement a transform that:

  • supports the full range of TS features (enums, constructor access modifiers, auto-stripping used-as-type-only imports... but decorators might be something worth more research)
  • preserves original line positions in all cases by generating runtime code in one line
  • preserves original column positions in most cases*

As an example, enum can be transformed as:

In

enum Foo {
  X = bar(),
  Y = X,
}

enum Foo {
  Z = X,
}

Out

var  Foo = ((Foo, X, Y) => (
  X = bar(),
  Y = X,
Foo[Foo.X = X] = "X", Foo[Foo.Y = Y] = "Y", Foo))({});

     Foo = ((Foo, {X, Y} = Foo, Z) => (
  Z = X,
Foo[Foo.Z = Z] = "Z", Foo))(Foo);

Class constructor access modifiers:

In:

class Foo {
  constructor(public: foo) {
    // ....
  }
}

Out:

class Foo {
  constructor(        foo) { this.foo = foo;
    // ....
  }
}

The rare cases where columns could be affected are cases where the user has code in the same line with the closing } of enums:

enum Foo {
  X = bar()
} doSomething()

...or the opening { of the constructor body:

class Foo {
  constructor(public: foo) { doSomething() }
}

Which I think should be rare in practice - so in practice there should be very few mapping entries that need to exist for source maps, greatly reducing the overhead. By narrowing the use case to this specific case only, the transform could also be written in a way that doesn't even need to construct an AST (just directly do source manipulation as it parses). The performance could be as fast, if not faster than the current swc strip-type-only implementation.

Would you say the existence of something like this will make the TSC more willing to consider full TypeScript syntax support?

@GeoffreyBooth
Copy link
Member

catering to such more general users seems to be the original goal of this effort.

I was the one who defined the original goal of this effort 😄 At least when we first starting discussing type-stripping a few weeks ago. The goal was to find something that we could ship within Node that would survive a three-year LTS period, despite TypeScript itself not following semver. At the time we assumed that TypeScript transforms would need to be ruled out because we assumed that they weren’t stable enough for a three-year lifespan, and fortunately the TypeScript team has informed us that that isn’t the case, so it’s an option to support them; but still, supporting them incurs a performance penalty. Perhaps less of a performance penalty if they can be transformed with the goal of trying to preserve line and column numbers, saving us from needing to generate a source map, but that’s a challenge for SWC and out of scope for Node.

Would you say the existence of something like this will make the TSC more willing to consider full TypeScript syntax support?

I can only speak for myself, not the TSC. I think you need to look at what some of the TypeScript team have been saying, though: “full” TypeScript support isn’t just type-stripping plus transforms, it’s also type checking and supporting all the options within tsconfig.json. Node can’t ever include all of that in core, because the type checking and tsconfig options very much do change within a 3-year cycle. So even if we support type stripping plus transforms, we’re still going to leave many users unhappy because of the other things that get left out.

And the solution for those users is to just use TypeScript in full, via tsc or --import=tsx or Vite or something similar. Those solutions already exist and will continue to exist, and Node will continue to support and encourage them; our error messages and docs will point people in their direction to get whatever features they want that Node can’t provide. Fundamentally it’s just not possible to satisfy both of the user requests of “full” TypeScript and no breaking changes for a three year LTS cycle. Either the TypeScript will get very out of date or it won’t be stable, and I consider both of those options to be worse than the two-option menu of type-stripping plus hooks for external tools.

I can imagine you asking “so if bundling the full TypeScript experience is off the table, what about the semi-full TypeScript of just type-stripping plus transforms?” And my reply is to just look at the work I did to migrate Corepack. Maybe 5% of it was dealing with the public class property and the enum, and I could’ve migrated the enum in a much simpler way than I did. Much more time was spent on updating import specifiers, though I’m sure that could be automated (and migrating most if not all syntaxes like enums could probably be automated too). In other words, transforms aren’t much of an issue, and they have modern equivalents that are strippable, so I think it’s a better tradeoff to ban them and get faster performance than support something that could be deprecated. It’s a certainty that any built-in TypeScript support that Node has won’t be able to run all existing TypeScript code, so if people are going to need to migrate at all (whether by hand or via a tool) they might as well migrate to the most performant syntaxes rather than Node sacrificing some performance to save some of the migration work. And finally, our target audience for this feature is users starting new projects, not users migrating existing ones, as the latter users by definition already have a solution that works for them. New users starting from scratch can work within the constraints of type stripping if that’s how they want to run their TypeScript, and presumably an ecosystem will grow with linting tools and the like for helping guide them. And for everyone else, as I keep saying, tsc and tsx and Vite and tsimp and ts-node and so on will continue to exist and continue to work, and continue providing fuller experiences than Node ever can.

@yyx990803
Copy link

yyx990803 commented Jul 14, 2024

First of all, by "full syntax support" I mean a transform-only support, like what esbuild / swc / oxc does with isolatedModules: true. Never intended to want type-checking, tsconfig options etc.

Perhaps less of a performance penalty if they can be transformed with the goal of trying to preserve line and column numbers, saving us from needing to generate a source map, but that’s a challenge for SWC and out of scope for Node.

It's not out of scope. If such transform exists with negligible perf overhead compared to strip-types-only, then the argument for Node.js to go with strip-types-only becomes much weaker. In fact, I am raising this question because I am very confident this is something feasible, therefore the assumption that "strip-types-only has big perf wins" needs to be challenged.

Out of curiosity I ran the same benchmark on your modified corepack source using oxc-node:

Benchmark 1: node --import @oxc-node/core/register ./corepack/sources/_cli.ts
  Time (mean ± σ):      93.4 ms ±   1.4 ms    [User: 99.8 ms, System: 13.4 ms]
  Range (min … max):    91.4 ms …  96.7 ms    30 runs

Benchmark 2: node --import tsx ./corepack/sources/_cli.ts
  Time (mean ± σ):     136.0 ms ±   0.8 ms    [User: 161.1 ms, System: 21.4 ms]
  Range (min … max):   134.0 ms … 137.5 ms    30 runs

Summary
  node --import @oxc-node/core/register ./corepack/sources/_cli.ts ran
    1.46 ± 0.02 times faster than node --import tsx ./corepack/sources/_cli.ts

The perf is 1.46x over tsx vs the 1.22x of current --experimental-strip-types. Granted this is native binary vs wasm, but this one is actually running a full transform with source maps, and even respecting resolution config in tsconfig.json. Even both as native binary, an oxc-based position-preserving + no AST allocation TS transform can very possibly out perform other strip-type-only implementations. SWC can also implement the proposed transform on top of the current strip-type-only implementation and compare the result.

In other words, transforms aren’t much of an issue

I fundamentally disagree with this. Removing these small discrepancies avoids a Node-flavored subset of TS. Right now these discrepancies are justified in the name of performance - but as I said, the assumption that "strip-types-only has big perf wins" needs to be challenged.

and presumably an ecosystem will grow with linting tools and the like for helping guide them.

This is encouraging yet another flavor of TS that only works in Node, creating more fragmentation / confusion in an already complicated landscape, but the other side of the tradeoff is questionable for the reasons outlined above.

"Node.js supports full syntax-level TS transforms, but not type checking / tsconfig" is a much cleaner definition than "Node.js supports a subset of TS syntax". It aligns with all the other transforms users are already using via esbuild / swc / oxc etc.

My whole argument is that it is not a good idea to intentionally steer away from possible ecosystem alignment in the name of performance, especially when the performance gain might not be as significant as imagined.

@jakebailey
Copy link

jakebailey commented Jul 14, 2024

I think you need to look at what some of the TypeScript team have been saying, though: “full” TypeScript support isn’t just type-stripping plus transforms, it’s also type checking and supporting all the options within tsconfig.json

I don't think anyone has actually advocated for checking; that is of course the bit that "doesn't follow semver", though the general scheme is that "any change could be a breaking change". The config defaults are what change in semver major, and the API is otherwise very stable. Not that using TS itself here is a goal; any third party emitter can handle TS code if it complies with isolatedModules.

I mean a transform-only support, like what esbuild / swc / oxc does with isolatedModules: true. Never intended to want type-checking, tsconfig options etc.

Note that there are a couple of options respected by those tools which affect emit, namely useDefineForClassFields (and target after a specific version) and experimentalDectorators. These aren't affected by isolatedModules, and I'm not sure what swc does at this point (I think it leaves define class field semantics on by default, differing to TSC?)

@GeoffreyBooth
Copy link
Member

My whole argument is that it is not a good idea to intentionally steer away from possible ecosystem alignment in the name of performance, especially when the performance gain might not be as significant as imagined.

Sure. Right now, though, the transpiler you describe that can do transforms yet still not need source maps doesn’t exist, as far as I know. If someone builds that, and we measure the performance difference and it’s truly negligible, then it’s something to consider. We can always add support for transforms later, even after this goes stable.

But my point about looking at the changeset for the Corepack migration was that transforms account for very little of it, and the rest will still remain even if we support transforms: adding file extensions, adding with { type: 'json' }, using import type, etc. There are also some potential migrations that Corepack didn’t need but that many apps would, of avoiding tsconfig paths and adjusting to ESM-syntax code running as ESM rather than being transpiled into CommonJS (so no global require intermixed with import/export, etc.). All of these Node-specific idiosyncracies would remain even if we start supporting transforms. Maybe some of them we might find ways to get rid of, but I doubt we’d eliminate all of them.

And so then yes, Node’s built-in support will support a particular subset of TypeScript’s full feature set, and users wanting to use that support instead of tsc or tsx or Vite etc will need to live within those limitations. Probably everything that would need to change could be automated, from adding file extensions to splitting import/import type to migrating from tsconfig paths to package.json "imports" and so on. And if that’s the case, I’m not sure why transforms are such a concern as compared with any of the other things that ecosystem tools might support that Node might not.

@yyx990803
Copy link

yyx990803 commented Jul 14, 2024

If someone builds that, and we measure the performance difference and it’s truly negligible, then it’s something to consider. We can always add support for transforms later, even after this goes stable.

Great. That's pretty much what I want to hear.


Surely there will be some migration costs that cannot be covered by transforms, but don't we agree that the smaller the differences are, the better?

More importantly we need to evaluate these differences in context - things like tsconfig paths are much easier to explain because Node.js' own module resolutions rules need to be the source of truth, but explaining that

class Foo {
  construct(public id: number) {}
}

...works as expected everywhere else but just not in Node.js (which claims to "support TypeScript") seems... very awkward.

@marco-ippolito
Copy link
Member Author

marco-ippolito commented Jul 14, 2024

@kdy1 do you think is possible to perform transformation of ts features and preserve the position in the file with whitespacing?

@arcanis
Copy link
Contributor

arcanis commented Jul 14, 2024

@arcanis I wanted to see how hard it would be to migrate an existing app to use --experimental-strip-types, so I migrated Corepack: https://github.com/nodejs/corepack/compare/main…GeoffreyBooth:corepack:strip-types

I didn't ask whether the migration was hard to someone well versed in Node.js. I did ask however:

  • Who is this feature built for? What is the intended audience?
  • Why is this feature designed by default as a departure from status quo?
  • Why have other tools been able to support transforms, and Node.js would be unable to?
  • Why is Node.js taking ownership of the syntax allowed in a .ts file?
  • What are examples of packages shipped to run as TS, and do they use enums?

As for whether the migration would be easy or hard, that matters much less to me than having a migration. As designed, the feature would force people out of specific TS syntax constructs, which seems an overreach to me.

@kdy1
Copy link
Member

kdy1 commented Jul 14, 2024

@marco-ippolito It's partially possible, but it's a bit hard in general.

Basic restrictions are

  • all transformed results have exactly the same order as the input syntax.
  • it should be at most lexing + parsing + single visit (because of the performance)

We need to evaluate each syntax under this assumption.

Enums

Enums are typically written as

enum Foo {
    a = 1,
    b = 2, c = 3
}

and it can be transpiled as

(function(Foo) {
    Foo[Foo["a"] = 1] = "a";
    Foo[Foo["b"] = 2] = "b"; Foo[Foo["c"] = 3] = "c"; 
})(Foo || (Foo = {}));

so it can be supported.

Decorators

The decorator pass is too complex to be supported with this kind of transpilation. There's no way to preserve token order.

Constructor properties

I'm not sure if this is possible or not, but my guess is that it's impossible in some cases, meaning trying to support it may result in forcing specific formatting for the users.

If you have some ideas, please share

@yyx990803
Copy link

yyx990803 commented Jul 14, 2024

@kdy1 I shared an example here: #208 (comment)

Since decorators are on standards track and already stage 3, I don’t think they need to be covered by the TS transform in the long run. Short term it may be the only exception.

@marco-ippolito
Copy link
Member Author

marco-ippolito commented Jul 14, 2024

I'm willing to support full ts features, behind a flag, if we see it works well and it's stable, we will as well remove the flag and set it to default.
Example:

node --expertimental-strip-types --enable-ts-transform  foo.ts

And with source maps support

node --expertimental-strip-types --enable-ts-transform  --enable-source-maps foo.ts

The current swc version does not support this so it's gonna be in a next iteration.
If we can make it work without source-maps and with whitespacing it would be even better

@GeoffreyBooth
Copy link
Member

GeoffreyBooth commented Jul 14, 2024

Who is this feature built for? What is the intended audience?

  • Users writing scripts without package.json or dependencies, where the user wants to use TypeScript syntax.
  • Users starting new projects where they can write using type stripping-compatible TypeScript syntax/settings from the start, who prefer the speed of built-in native compilation. (We're using Wasm SWC currently, but the intention is to replace it with a native build once we can figure out how to get Rust into our build chain.)

Why is this feature designed by default as a departure from status quo?

  • Node already supports the “status quo”, via --import=tsx.
  • One of the primary reasons that anything gets included in core as opposed to being left to userland is because the feature can be faster when included in core, usually by being implemented using native code. Especially since TypeScript via hooks is already possible, and that supports the TypeScript that users are already familiar with, this built-in support should be optimized for performance.
  • The proposed behavior matches the Type Annotations proposal, which might become a future status quo (and which many of us support). If that occurs, then type stripping will just become standard JavaScript, and code written for type stripping today will work in any runtime including browsers; and Node will be able to simplify our internals by removing SWC, since V8 would handle the type stripping for us.

Why have other tools been able to support transforms, and Node.js would be unable to?

  • Node already supports transforms, via --import=tsx.
  • Node could support transforms via its built-in support as well, either enabled all the time or via a separate opt-in flag, but I think we would want to do so only if performance were not noticeably impacted. A main advantage of the built-in support is theoretically improved performance, so we wouldn’t want to noticeably degrade that to support features that users can already access via --import=tsx. Or put another way: if type stripping lands, Node will have two methods for supporting TypeScript. It’s better if one of them is fast, even if slightly limited, rather than both of them being slow and equivalent.
  • Transforms are not part of the Type Annotations proposal, so supporting them muddies the transition to that potential future where that proposal graduates. Since transforms are already supported via --import=tsx, it might be preferable to keep type-stripping aligned with that proposal as it advances, at least until it either graduates or is abandoned.

Why is Node.js taking ownership of the syntax allowed in a .ts file?

  • Because no one else is? Has there been a proposal from the TypeScript team for adding built-in support of TypeScript in Node?
  • Type stripping is primarily inspired by the Type Annotations proposal, of which TypeScript’s @DanielRosenwasser is a champion. This is arguably just as blessed a syntax or standard as the moving target of whatever a given version of tsc supports.

What are examples of packages shipped to run as TS, and do they use enums?

I don’t know of any. As the TypeScript team has explained, shipping packages written in TypeScript is generally a bad idea, unless you’re in control of your consumers (for example, if it’s meant for use only by you in your own app, or for the members of your team). Even if there are packages out there that don’t use enums, they probably don’t follow all of the rules of --experimental-type-stripping (see the things I changed in Corepack). The goal isn’t to support existing published packages written in TypeScript, if there are any.

@jakebailey
Copy link

The proposed behavior matches the Type Annotations proposal, which might become a future status quo (and which many of us support). If that occurs, then type stripping will just become standard JavaScript, and code written for type stripping today will work in any runtime including browsers; and Node will be able to simplify our internals by removing SWC, since V8 would handle the type stripping for us.

Type stripping is primarily inspired by the Type Annotations proposal, of which TypeScript’s @DanielRosenwasser is a champion. This is arguably just as blessed a syntax or standard as the moving target of whatever a given version of tsc supports.

I would still shy away from this as a motivator; the Type Annotations proposal may not cover all TS syntax. Not only that, but it can't, because in TypeScript the expression foo<number>(value) is a function call with type arguments, but in JavaScript that's two comparisons. This means that the type annotations proposal will never contain all valid TypeScript syntax today. As such --experimental-strip-types would a different subset of the language (so not "matching" the TC39 proposal), and can't be eventually replaced by something from V8.

@marco-ippolito
Copy link
Member Author

Since this has landed and we have a roadmap I'll close this issue, we can talk about next steps here #217

@thesoftwarephilosopher
Copy link

@marco-ippolito I want to help as much as I can to get this feature working thoroughly before 2025. I have nothing but free time. Please let me know how I can help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
typescript Discussions related to TypeScript
Projects
None yet
Development

No branches or pull requests