tape-testing / tape

tap-producing test harness for node and browsers
MIT License
5.77k stars 307 forks source link

Using typescript with esm leads to importOrRequire issue #577

Closed ryanio closed 1 year ago

ryanio commented 2 years ago

Hey tape, thanks for the wonderful project we use it in all our repositories at ethereumjs :)

I am updating our TypeScript repositories from commonjs to esm and having some trouble with the importOrRequire function when running our tests.

I couldn't find a way to pass the esm loader to tape, so I am using the env NODE_OPTIONS='--loader ts-node/esm' tape test/*.spec.ts, but then the importOrRequire function gets the path with .ts, so it uses require() and errors out with Must use import to load ES Module:

https://github.com/substack/tape/blob/12cc602f5296b023f5e226c946192e5aea453252/bin/import-or-require.js#L11-L14

For now I put a small hot fix in our postinstall script to work for '.ts', but wondering if there is a better way to accomplish what I am trying, or if this is fine then I am happy to open a PR to add '.ts' to the above if statement. After that all our tests are running and passing successfully :)

ljharb commented 2 years ago

we use it in all our repositories at ethereumjs

Glad to hear it!

ESM loaders are still experimental, and thus we haven't done any work to support them yet. I'm not sure how we'd determine that a given file is indeed destined for the loader - node doesn't support .ts files, and it could be a CJS TS file (using ts-register) or an ESM TS file (using the loader).

So, how would tape know that a file that node wouldn't otherwise support should be supported, and which module type it is?

ryanio commented 2 years ago

it's a great question. I think the getPackageType.sync(file) === 'module' works well, just have to add typescript file extensions since the loader is passing them directly? So I guess that would be ts, tsx, mts (https://www.typescriptlang.org/docs/handbook/esm-node.html#new-file-extensions) cjs/cts should fall through to require.

ljharb commented 2 years ago

getPackageType is for node standard stuff tho, so it wouldn't be quite right for it to support TS (it's not my package, of course)

cmdruid commented 1 year ago

Hello. I am running into this problem as well. Having "type": "module" in package.json should mean that .js/.ts files are modules by default, but tape import loader uses a different logic, and breaks when this declaration is made.

ljharb commented 1 year ago

Yes, I agree that type module causes .js files to be ESM by default, which getPackageType should support, but .ts files shouldn't ever work since they're not ever importable (until ESM loaders are a non-experimental thing).