-
Notifications
You must be signed in to change notification settings - Fork 17
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
Comments
I have two concerns (none of them blocking):
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" |
To answer to your points:
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 |
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 |
+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. |
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 |
Not against renaming the flag, but for this very limited features that we are adding, |
This was discussed today in the loaders meeting:
|
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 |
How can this be used to just strip types and nothing more? |
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. |
It's not It
These operations is one |
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:
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 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. |
**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)
@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. |
@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. |
My view on this
I'm happy ok with renaming it |
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. |
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 |
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 |
currently my pr already supports all of that with the version of swc we are using. BUT it's not real type-stripping. |
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 |
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 |
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. |
This is subjective but:
|
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.
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. 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
JSR and Deno were brought up in this context, but they are "special" in that for non-Deno users, JSR serves up
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 |
This is already possible today:
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.
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
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 |
I vehemently disagree and I will block this to the utmost of my ability. |
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 |
Yes, in my mind everything is just transforms. 😄 (This discussion was the first I had head the term "type stripping"; in
I think the reason it's getting brought up from different angles is just due to this being motivated by |
@arcanis I wanted to see how hard it would be to migrate an existing app to use I got it as far as being able to print the help text when run with # 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:
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 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 I mentioned in nodejs/node#53725 (comment) that we should hold off on releasing I also did a quick benchmark using the same 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 |
@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:
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 enum Foo {
X = bar()
} doSomething() ...or the opening 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? |
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.
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 And the solution for those users is to just use TypeScript in full, via 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, |
First of all, by "full syntax support" I mean a transform-only support, like what esbuild / swc / oxc does with
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:
The perf is 1.46x over
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.
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. |
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.
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?) |
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 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 |
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. |
@kdy1 do you think is possible to perform transformation of ts features and preserve the position in the file with whitespacing? |
I didn't ask whether the migration was hard to someone well versed in Node.js. I did ask however:
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. |
@marco-ippolito It's partially possible, but it's a bit hard in general. Basic restrictions are
We need to evaluate each syntax under this assumption. EnumsEnums 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. DecoratorsThe decorator pass is too complex to be supported with this kind of transpilation. There's no way to preserve token order. Constructor propertiesI'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 |
@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. |
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. 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. |
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 |
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 |
Since this has landed and we have a roadmap I'll close this issue, we can talk about next steps here #217 |
@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. |
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:
.cts
.js
because of transpilation => but at runtime is a TS).ts
files in the node_modulesRoadmap: #217
The text was updated successfully, but these errors were encountered: