-
-
Notifications
You must be signed in to change notification settings - Fork 33.7k
module: mark type stripping as stable #60600
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -226,7 +226,7 @@ You can find their contact email in the [`README.md`](../../README.md#tsc-techni | |
|
|
||
| ##### Goal | ||
|
|
||
| The goal is to raise awareness of the Node.js TypeScript Type Stripping in the JavaScript ecosystem. | ||
| The goal is to raise awareness of the Node.js TypeScript Type stripping in the JavaScript ecosystem. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: Why capitalise "Type" but not "stripping"?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Idk 😆 I've read "Type Stripping", "Type stripping", "type-stripping" not sure which one is correct
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it should be "type stripping", personally /ducks |
||
| Some of the things to highlight include: | ||
|
|
||
| * The benefits and limitations of the current implementation. | ||
|
|
@@ -271,7 +271,7 @@ with projects learning from one another and their users. | |
| | [reading environment](https://nodejs.org/api/cli.html#--env-filefile) | [20.6.0](https://nodejs.org/en/blog/release/v20.6.0) | Active Development | | ||
| | [styling output](https://nodejs.org/docs/latest-v22.x/api/util.html#utilstyletextformat-text-options) | [20.12.0](https://nodejs.org/en/blog/release/v20.12.0) | Stable, as of [22.13.0](https://github.com/nodejs/node/pull/56329) | | ||
| | [run scripts](https://nodejs.org/docs/latest/api/cli.html#--run) | [22.0.0](https://nodejs.org/en/blog/release/v22.0.0) | Stable, as of 22.0.0 | | ||
| | [run TypeScript](https://nodejs.org/api/cli.html#--experimental-strip-types) | [22.6.0](https://nodejs.org/en/blog/release/v22.6.0) | Active Development | | ||
| | [run TypeScript](https://nodejs.org/api/cli.html#--strip-types) | [22.6.0](https://nodejs.org/en/blog/release/v22.6.0) | Stable, as of REPLACEME | | ||
|
|
||
| ##### Related Links | ||
|
|
||
|
|
@@ -284,7 +284,7 @@ with projects learning from one another and their users. | |
| * <https://nodejs.org/api/cli.html#--env-filefile> | ||
| * <https://nodejs.org/docs/latest-v22.x/api/util.html#utilstyletextformat-text-options> | ||
| * <https://nodejs.org/api/cli.html#--run> | ||
| * <https://nodejs.org/api/cli.html#--experimental-strip-types> | ||
| * <https://nodejs.org/api/cli.html#--strip-types> | ||
|
|
||
| <!-- lint enable prohibited-strings remark-lint--> | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the goal is to keep a stable, long-term flag to disable type stripping? Is this something that users want?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not a long term goal in the sense I wish there was no need to disable it, but given the ecosystem monkeypatches internal etc and some packages break... it's necessary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we keep the disabling flag as experimental? I don't think we gain anything from migrating that to stable, even though we don't forsee getting rid of it just yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you mean rename it
--no-strip-typesbut keep it experimental?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's the thing, I wouldn't rename it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we want type stripping to be stable, it should feel weird to disable it. Keeping the
experimentalin the flag does a good job at that – albeit the word order is a bit off, but IMO that's acceptable. I'm not against adding the alias withoutexperimentalin the name, I don't think it sends the right signal is all.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I kinda agree with your point of view, the fact that can be disabled it's not something we want to drag for too long and the flag is there only for people who probably are using some library that does some monkeypatching or wrong assumption and it breaks. We have to temporarly provide users a way to disabled, we will eventually get rid of it. So
--no-experimental-strip-typesstays but at the same time it feels weird to call it experimental, since we mark it as stable. Whats unstable is the way to disable it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My suggested approach is to remove experimental from the name, and in a future semver major, make it a no-op
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 to drop the experimental prefix but I am -1 to make it a no-op in the future. That would leave no room for people getting broken to run their code on newer versions of Node.js (I don't think it's just ts-node, the other day I was running the tests of tsx and on 22 there was a regression from 20 that would go away if I disable type stripping).
I don't think having disabling option is weird for a stable feature, we have many disabling flags in Node.js for stable features. Bugs and gotchas happen and people need them to work around whatever issues their setup has with the default behavior. That's fine, the default may work for 99% of the people but there can always be 1% that really have to disable it to be able to run their code at all. For example some time ago I added --disable-wasm-trap-handler because in some environments, the WASM trap handler would make it impossible to run WASM at all. But WASM trap-based bound checks should still be considered stable and perfectly fine to use to provide significant speed up for WASM code. Its design just unfortunately doesn't work in certain niche environments and people have to accept the slowdown for keeping their app functional at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you meant "make
--strip-typesa no-op, then sure that can be discussed; if you mean "make--no-strip-typesa no-op, that would be quite weird, it's not how we usually handles such flags (e.g.--experimental-top-level-awaitis still accepted,--no-experimental-top-level-awaitthrows).But we're not dropping the experimental one, we're adding a new flag that has the same effect, so now we have two.
Anyway, either way is fine, it's not like the additional flag would be a maintenance burden, and I don't feel strongly about the name.