mochajs / mocha

☕️ simple, flexible, fun javascript test framework for node.js & the browser
https://mochajs.org
MIT License
22.6k stars 3.01k forks source link

Can't use node-option with ts-node #5047

Closed ericvergnaud closed 9 months ago

ericvergnaud commented 10 months ago

Prerequisites

Description

when running mocha with both ts-node and node-option parameters, ts-node file extensions (.ts) are no longer supported

Steps to Reproduce

Expected behavior:

I expect the test to run

Actual behavior:

When running the above, I get a TypeError: Unknown file extension ".ts" If I remove --node-option experimental-wasm-gc I don't get the error If I run node --experimental-wasm-gc node_modules/mocha/bin/mocha.js --no-warnings --loader=ts-node/esm --require ts-node/register --ui bdd src/test.spec.ts, then the test complains that the experimental-wasm-gc option is missing (I'm loading a WebAssembly)

Reproduces how often:

Always

Versions

Additional Information

ericvergnaud commented 10 months ago

It seems the issue could be located in mocha.js lines 84-86, where the presence of node-option drops the already parsed node args (in nodeArgs variable) instead of combining them

JoshuaKGoldberg commented 9 months ago

Curiosity: why provide both --loader=ts-node/esm and --require=ts-node/register?

The list of steps provided seems reasonable, but I'm not getting the exact same output locally. Could you please post an isolated reproduction that a maintainer can run with only 1-2 commands, such as a GitHub repository, GitHub Gist, or Stackbliz container?

ericvergnaud commented 9 months ago

you need ts-node/register to compile ts on the fly you need ts-node/esm to use esm module syntax

Here is an example MochaBug.zip

JoshuaKGoldberg commented 9 months ago

Got it, thanks! Makes sense. +1 to your (very helpful, thanks!) comment that Mocha is overriding the implicit-ish --loader=ts-node Node option with the explicit --node-option experimental-wasm-gc. If you pass in --node-option loader=ts-node/esm instead, then everything works happily.

$ node node_modules/mocha/bin/mocha.js --no-warnings --require ts-node/register --ui bdd --node-option loader=ts-node/esm --node-option experimental-wasm-gc src/test.spec.ts                     
(node:37672) ExperimentalWarning: `--experimental-loader` may be removed in the future; instead use `register()`:
--import 'data:text/javascript,import { register } from "node:module"; import { pathToFileURL } from "node:url"; register("ts-node/esm", pathToFileURL("./"));'
(Use `node --trace-warnings ...` to show where the warning was created)

Hi there!
  ✔ runs a test

  1 passing (1ms)

...where "happily" ignores the loader notice. That's discussed in #5002.

I think we can consider this a docs issue similar to #5002 that it's not clear how to use Node import/loader/require options together. Filed #5087 to track adding docs. It links to https://github.com/mochajs/mocha-examples/pull/76 as a start.

Aside: in general, it's not great practice to upload a .zip. Downloading and extracting arbitrary files from the internet is kind of scary for us maintainers 🫠. An isolated GitHub repository, Gist, Stackblitz, or other more transparent environment would be nice in the future. This zip works for now.

Thanks for filing!