-
-
Notifications
You must be signed in to change notification settings - Fork 75
refactor: leverage process.features.typescript instead of node version #452
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
refactor: leverage process.features.typescript instead of node version #452
Conversation
const runtime = require('../lib/runtime') | ||
|
||
const nodeVersion = Number(process.version.split('.')[0].slice(1)) |
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 removed nodeVersion
from runtime because it is only used here and prevent to reach 100% coverage on the main test suite.
@@ -1,6 +1,6 @@ | |||
'use script' | |||
|
|||
const t = require('tap') |
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.
Weird that it didn't fail previously 🤔
looks great, just one thing. |
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.
lgtm
This is a good fix, but leaving a hint here for anyone else coming across this: In autoloader v6.2.0 or later, autoloader can directly load .ts files, if they are are compatible with native Node.js type stripping, however, if you run the autolodaer in the node-tap test runner with the typescript plugin removed, e.g.
the autoloader will throw an error about not being able to run typescript directly. To get around this, you can re-enable node-taps typescript plugin and disable typechecking if you don't want it to do anything.
I'm thinking this is something node-tap should fix rather than something addressed here, but in case you come across the same random situation, there is a solution. |
Node version is not reliable enough to guess typescript support, but we can leverage
process.features.typescript
.fastify/fastify-cli#798 (comment)