Update for v5 #371
Update for v5 #371
Conversation
test/commonjs/error.js
Outdated
| // TODO: fix this test | ||
| // const app3 = fastify() | ||
|
|
||
| app3.register(require('./ts-error/app')) | ||
| // await app3.register(require('./ts-error/app')) | ||
|
|
||
| app3.ready(function (err) { | ||
| t.type(err, Error) | ||
| t.match(err.message, /cannot import plugin.*typescript/i) | ||
| }) | ||
| // app3.ready(function (err) { | ||
| // t.type(err, Error) | ||
| // t.match(err.message, /cannot import plugin.*typescript/i) | ||
| // }) |
There was a problem hiding this comment.
This no longer errors, I have no idea why
This file is supposed to cause an error, but don't know why:
https://github.com/fastify/fastify-autoload/tree/master/test/commonjs/ts-error
There was a problem hiding this comment.
Just asking since you are contributing to tests, do you have any idea why this could be happening?
There was a problem hiding this comment.
Once the TS error is fixed, there is a loader error:
Error: tsx must be loaded with --import instead of --loader
The --loader flag was deprecated in Node v20.6.0
at X (file:///home/runner/work/fastify-autoload/fastify-autoload/node_modules/tsx/dist/esm/index.mjs:1:1920)
at Hooks.addCustomLoader (node:internal/modules/esm/hooks:202:24)
at Hooks.register (node:internal/modules/esm/hooks:168:16)
at async initializeHooks (node:internal/modules/esm/utils:167:5)
at async customizedModuleWorker (node:internal/modules/esm/worker:104:24)
```There was a problem hiding this comment.
(18, ubuntu-latest) test fail, 16 is fine: https://github.com/jean-michelet/fastify-autoload/actions/runs/8214240315/job/22466499571#step:5:793
Wasn't sure what to do here... But I can try to fix it next week.
There was a problem hiding this comment.
Sorry @gurgunday ,
I answered too quickly and didn't notice your work on your first commit.
Have you made any progress?
This file is supposed to cause an error, but don't know why:
https://github.com/fastify/fastify-autoload/tree/master/test/commonjs/ts-error
Maybe it should throw an error because autoloader try to load a file with .ts extension (lib/a.ts) here:
I can get your work in the first commit and try to fix it in #370, wdyt?
| const app3 = fastify() | ||
| // TODO: Fix this test | ||
| // const app3 = fastify() | ||
| // app3.register(require('./ts-error/app')) |
| // app3.ready(function (err) { | ||
| // t.type(err, Error) | ||
| // t.match(err.message, /cannot import plugin.*typescript/i) | ||
| // }) |
|
We can't merge without that test passing |
|
When you run the relevant tests directly with Node using If you log this here: console.log(typescriptSupport, isTsNode)You'll notice the behavior: they are both By the way, it seems that it doesn't check types, only transpiles them. I guess it's the expected behavior? It transpiles fine absurd typing: 'use strict';
module.exports = function (f: void, opts: void, next: void) {
f.get('/something', (request, reply) => {
reply.send({ something: 'else' });
});
next();
}; |
|
I'm not sure if I'll have time to address it this week, but it appears that the upgrade to Tap 18 is responsible for this behavior. They have removed some default options, as mentioned here. I managed to get the test to pass by removing the TypeScript plugin I hope this helps you see things more clearly ^^ |
| @@ -4,9 +4,9 @@ const { exec } = require('node:child_process') | |||
|
|
|||
| const args = [ | |||
| 'tap', | |||
| '--node-arg=--loader=ts-node/esm', | |||
| '--node-arg=--import=ts-node/esm', | |||
There was a problem hiding this comment.
Are you sure --import=ts-node/esm will works?
There was a problem hiding this comment.
It works because tap handle the transpile, you can see it run even if you remove --node-arg=--import=ts-node/esm
|
Is it possible to force The tests would pass with small modifications: jean-michelet#6 // scripts/unit.js
'use strict'
const { exec } = require('node:child_process')
doExec('npm run unit:with-modules', {
shell: true,
env: {
...process.env,
FASTIFY_AUTOLOAD_TYPESCRIPT: 0
}
})
doExec('npm run unit:with-ts-modules')
function doExec (cmd, opts = {}) {
const child = exec(cmd, opts)
child.stdout.pipe(process.stdout)
child.stderr.pipe(process.stderr)
child.once('close', process.exit)
}// index.js
let typescriptSupport = false
if (Number(process.env.FASTIFY_AUTOLOAD_TYPESCRIPT) !== 0) {
typescriptSupport = isFastifyAutoloadTypescriptOverride || isTsNode || isVitestEnvironment || isBabelNode || isJestEnvironment || isSWCRegister || isSWCNodeRegister || isSWCNode || isTsm || isTsx || isEsbuildRegister
} |
|
@jean-michelet actually would you like to take over? You can just point your branch to |
Not that easy now that #370 have been merged... But, okay, I'll do it next week. |
No description provided.