Run TypeScript scripts directly#2322
Conversation
Also removes enums. There's `--experimental-transform-types` for that but that's still experimental and TS team regretted adding enums after all.
This isn't really true 😄 |
jakebailey
left a comment
jakebailey
left a comment
There was a problem hiding this comment.
I can't properly review this at the moment but should be able to do so in a few days
There was a problem hiding this comment.
We should really just move these to src tbh
There was a problem hiding this comment.
I can't properly review this at the moment but should be able to do so in a few days
There was a problem hiding this comment.
That's not strictly needed here and can happen separately IMO
That's what I'm told in some decades old issue, are things changed? |
| InstanceOnly, | ||
| All, | ||
| } | ||
| type EmitScope = "StaticOnly" | "InstanceOnly" | "All"; |
There was a problem hiding this comment.
I'd just use an as const object and the keyof typeof trick
There was a problem hiding this comment.
Hmmmmmmmmmmmmmmmm..................... I kinda prefer this way as it's simpler and it's what we do in web APIs 🤔
There was a problem hiding this comment.
Sure, but this was a TS enum before, so IMO this is easier:
const EmitScope = {
StaticOnly: "StaticOnly",
InstanceOnly: "InstanceOnly",
All: "All",
} as const;
type EmitScope = typeof EmitScope[keyof typeof EmitScope];Then no other code has to change.
But, the strings aren't the worst either
tsconfig.json
Outdated
| "noUnusedLocals": true, | ||
| "noUnusedParameters": true, | ||
| "resolveJsonModule": true, | ||
| "allowImportingTsExtensions": true, |
There was a problem hiding this comment.
We should set these to match https://nodejs.org/api/typescript.html#type-stripping
jakebailey
left a comment
jakebailey
left a comment
There was a problem hiding this comment.
This PR seems fine to me, just has conflicts with main
|
LGTM |
|
Sorry @saschanaz, you don't have access to these files: |
|
Ah, duh |
3 participants
Also removes enums. There's
--experimental-transform-typesfor that but that's still experimental and TS team regretted adding enums after all.Fixes #2321
Closes #2125