Skip to content

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

Merged
merged 4 commits into from
Mar 10, 2025

Conversation

jean-michelet
Copy link
Member

Node version is not reliable enough to guess typescript support, but we can leverage process.features.typescript.

fastify/fastify-cli#798 (comment)

Comment on lines -4 to +5
const runtime = require('../lib/runtime')

const nodeVersion = Number(process.version.split('.')[0].slice(1))
Copy link
Member Author

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')
Copy link
Member Author

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 🤔

@mcollina
Copy link
Member

mcollina commented Mar 9, 2025

looks great, just one thing.

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mcollina mcollina merged commit face997 into fastify:main Mar 10, 2025
11 checks passed
@bcomnes
Copy link

bcomnes commented Mar 11, 2025

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.

 "tap": {
"plugin": [
      "!@tapjs/typescript"
    ]
}

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.

 "tap": {
    "typecheck": false
}

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.

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

Successfully merging this pull request may close these issues.

3 participants