avajs / typescript

Test TypeScript projects using AVA.
MIT License
73 stars 16 forks source link

ts-node improvements for ava projects? #29

Open cspotcode opened 3 years ago

cspotcode commented 3 years ago

I came across https://github.com/avajs/ava/issues/1109 which lists several criticisms of ts-node and predates some recent improvements. For example, ts-node has since implemented an ESM loader which I believe can handle #5.

20 -- source-map-support -- is enabled out-of-the-box in ts-node.

We allow all configuration to be placed in your tsconfig.json. And we have an swc integration that will be in the next release, allowing users to opt-in to much faster compilation than we can get with babel or typescript.

All that to say, if you have time, I would love to talk about any other bugs , limitations, or inconveniences that your users hit with ts-node. At the very least it will give me some useful data points for improving ts-node. A big goal of mine is to make TS and node tooling feel cleaner, easier, and require less boilerplate.

If you have any thoughts, please let me know.

novemberborn commented 3 years ago

Hi @cspotcode, that's great to hear!

It'd be great to add ts-node integration to this package. If you look at https://github.com/avajs/typescript/pull/12, perhaps we could have a compile: 'tsc' | 'ts-node' option?

What would be your ideal integration for both CJS and ESM outputs?

cspotcode commented 3 years ago

Interesting, what is the motivation for #12? Is it to leverage incremental compilation or to support project references? Or is it to make sure typechecking output behaves identically to tsc rather than worrying that ts-node will output slightly different diagnostics?

If ava launches workers with --loader ts-node/esm and with TS_NODE_PROJECT pointing to the right tsconfig.json, then I think ts-node will handle both CommonJS and ESM transpilation. So that could be our ideal integration.

However, to truly match tsc, ts-node needs to make a few behavioral changes: a) add support for incremental compilation and project references. I have some tickets and have done some of the research to get this done b) Encourage or make --files the default so that we don't mess with "files" and "include" arrays. Defaulting it has been requested on the TypeScript Discord many times. Without it can be faster in some situations but also causes confusion. c) Have an option to emit all diagnostics up-front instead of filtering per-file. Currently, we throw TS errors for a particular file when it is require()d. This matches node's behavior of throwing for SyntaxErrors, so it kinda fits the philosophy of marrying TS and node, but this makes our error reporting different than tsc

novemberborn commented 3 years ago

Interesting, what is the motivation for #12?

For me personally, I run tsc as a separate process. #12 would allow that compilation to take place when you invoke AVA itself.

If ava launches workers with --loader ts-node/esm and with TS_NODE_PROJECT pointing to the right tsconfig.json, then I think ts-node will handle both CommonJS and ESM transpilation. So that could be our ideal integration.

Sounds like one feature we need is the ability to configure a loader. Do you know if this can be done programmatically?

Presumably setting the environment variable programmatically is fine, as long as that's done before any loader code is loaded?

Have an option to emit all diagnostics up-front instead of filtering per-file. Currently, we throw TS errors for a particular file when it is require()d. This matches node's behavior of throwing for SyntaxErrors, so it kinda fits the philosophy of marrying TS and node, but this makes our error reporting different than tsc

This current behavior is more suited to our use case here though. One open question I have with #12 is where those diagnostics go when they're emitted during a test run.

szmarczak commented 3 years ago

I can pick this up once I'm done with #12 :heart:

szmarczak commented 3 years ago

where those diagnostics go when they're emitted during a test run.

Here's a text of what happens if compilation fails ``` ⚠ Experiments are enabled. These are unsupported and may change or be removed at any time. ✖ Internal error Error: Command failed with exit code 1: tsc --incremental test/agent.ts:128:1 - error TS2304: Cannot find name 'a'. 128 a ~ Found 1 error. Error: Command failed with exit code 1: tsc --incremental test/agent.ts:128:1 - error TS2304: Cannot find name 'a'. 128 a ~ Found 1 error. at makeError (/home/szm/Desktop/got/node_modules/execa/lib/error.js:59:11) at handlePromise (/home/szm/Desktop/got/node_modules/execa/index.js:114:26) at processTicksAndRejections (internal/process/task_queues.js:97:5) at async Object.compile (/home/szm/Desktop/got/node_modules/@ava/typescript/index.js:101:7) at async /home/szm/Desktop/got/node_modules/ava/lib/api.js:190:19 at async Promise.all (index 0) at async Api.run (/home/szm/Desktop/got/node_modules/ava/lib/api.js:189:28) at async Object.exports.run (/home/szm/Desktop/got/node_modules/ava/lib/cli.js:469:21) ─ ```
Here's a screenshot ![image](https://user-images.githubusercontent.com/36894700/114632504-1d592480-9cbf-11eb-9df7-fb8193467073.png)
szmarczak commented 3 years ago

https://nodejs.org/api/cli.html#cli_experimental_loader_module

Sounds like one feature we need is the ability to configure a loader. Do you know if this can be done programmatically?

I'm not 100% sure, but I doubt this :( I think we'll have to pass the --loader then.

cspotcode commented 3 years ago

Correct, today it cannot be done programmatically. I believe that node should add this feature, and hopefully pressure from projects like ours will encourage them to do so.

cspotcode commented 3 years ago

Another wrinkle is that, today, --loader ts-node/esm runs in the main thread, so we can automatically install our CommonJS loader at the same time. On modern node, --loader ts-node/esm is enough to enable both ESM and CommonJS hooks.

However, node is investigating moving the ESM loaders into another thread. When they do that, it may or may not break some workflows.

Just something to be aware of. It's one of the reasons I keep warning users that native ESM loader stuff is experimental.

cspotcode commented 3 years ago

I've been revamping ts-node's documentation. It's still very much a work-in-progress, but I've added a "Recipes" section inspired by yours and added a page for Ava.

https://typestrong.org/ts-node/docs/recipes/ava

I realize this duplicates information across Ava's and ts-node's documentation, but I figure it's better for users. Let me know if you have any corrections.

novemberborn commented 3 years ago

We could add a provider feature in AVA itself, so that, from here, you could configure a Node.js argument for the child process / worker thread that the tests are run in.

Given the highly experimental nature we could name this compile option experimental-ts-node?


I've been revamping ts-node's documentation. It's still very much a work-in-progress, but I've added a "Recipes" section inspired by yours and added a page for Ava.

typestrong.org/ts-node/docs/recipes/ava

That's great!

vjpr commented 3 years ago

Is there a recipe for using swc + ES modules?

cspotcode commented 3 years ago

@vjpr You want to combine ts-node's swc mode with the ESM loader?