ESM should require file extensions

[SimenB]

🐛 Bug Report

import t from './file';
import d from './directory/'

These should both fail to resolve, but since we use the same resolver as for CJS, it doesn't, and the import will work while it would have failed at runtime. This is also the case for dynamic imports.

This might need some support in resolve whenever they add support for ESM? browserify/resolve#222

Place where we call resolve in jest: https://github.com/facebook/jest/blob/c024dec130d9914dcc3418ea74c26f667db3dbfa/packages/jest-runtime/src/index.ts#L395-L398
https://github.com/facebook/jest/blob/c024dec130d9914dcc3418ea74c26f667db3dbfa/packages/jest-runtime/src/index.ts#L423

Node docs: https://nodejs.org/api/esm.html#esm_mandatory_file_extensions
Node issue for better errors: nodejs/node#30603

[aledalgrande]

Would this change fail also when --es-module-specifier-resolution=node is used?

[SimenB]

--experimental-specifier-resolution=node is the flag. And yeah, we'd need to keep the logic around for that case... Not sure how to best detect if it has been passed. It's not present in process.argv so we might need to have our own option for this 🙁

[nicolo-ribaudo]

Maybe you can get it from process.execArgv.

[cspotcode]

For what it's worth, ts-node is doing this today. We check both positional args and also the NODE_OPTIONS environment variable. If you would like, I'll point you at our implementation. We could also expose it via our API surface.

There's probably going to be a fair bit of overlap here, both for this and for mapping between rootDir and outDir and between file extensions. Let me know if collaborating on a shared module to handle this stuff makes sense to you.

[mikicho]

I'm using eslint-plugin-import with "import/extensions": ["error", "ignorePackages"] rule to make sure all imports have extensions

[SimenB]

OK, now that TS has announced they'll be forcing Node ESM to use extensions (https://devblogs.microsoft.com/typescript/announcing-typescript-4-5-beta/#new-file-extensions, https://twitter.com/orta/status/1444958295865937920) we need to figure this out (preferably releasing a few weeks before 4.5 goes stable to get ahead of all the people who will complain that Jest doesn't work).

---

Current plan is to enforce file extensions for ESM resolution (import and import()), but allow some sort of opt-in to the current (i.e. CJS) behavior (like --experimental-specifier-resolution mentioned above does). I don't think I'd like to auto-detect the presence of that flag.

This also impacts https://jestjs.io/docs/configuration#extensionstotreatasesm-arraystring - we might have to start throwing on that and tell people to use type...

/cc @ahnpnl

[SimenB]

@cspotcode I'd be very interested to hear how this impacts ts-node 🙂

[cspotcode]

The file extensions behaviour shouldn't affect ts-node much. We already align with TS and node as far as file extensions go, and we resolve correctly.

import statements should point to the emitted js (that's what you want to import, after all) and we resolve that to the source .ts, since it's our job to essential emit in-memory, not on the filesystem. For production you can pre-compile to the filesystem and get rid of ts-node and everything works.

ts-node matches node's behavior regarding --experimental-specifier-resolution, which is to say, if you omit that flag, then file extensions are required. If you pass that flag via NODE_OPTIONS or whatever, then you get the behavior of that flag.

[ahnpnl]

Currently, ts-jest logic follows what Jest asks for. IIRC extensiosToTreatAsESM is a short-term solution to adopt ESM? If we can switch to use type instead, it would be better that we can remove one Jest config option.

In general, for transformer, I think we should just stick to the current approach that transformer should give what Jest needs. Maybe small changes from ts-jest side.

[SimenB]

I don't think we can remove extensionsToTreatAsESM, if nothing else since TS doesn't have any plans (I asked Orta) for enforcing extensions when outputting ESM for non-node (even though it's technically invalid ESM, they cannot break everybody outputting it and then consumed by bundlers). So Jest shouldn't force people to use extensions if they don't want. extensionsToTreatAsESM is a nice escape hatch for those cases.

But we should probably expand our current handling to first check extensionsToTreatAsESM, then look at type also for .ts and .tsx.

But defaulting to requiring the extension for ESM mode seems sensible to me. Then maybe some way of specifying you want CJS resolution mode (I don't wanna sniff out options passed to node, I'd rather have a CLI options ourselves - people can toggle that on or off via looking at node flags if they want)

[cspotcode]

even though it's technically invalid ESM

Isn't the format of module specifiers host-defined, so it's valid ESM? According to the spec.

[SimenB]

Sure, but it's invalid in node (without experimental flag) and browsers, which should cover (completely made up number) 99% of hosts running the code natively, no?

[cspotcode]

Bundlers are effectively a runtime target. I simply mean that, in terms of the TS team's decision-making, it is technically valid ESM, which is why they're supporting it. It's not merely a backwards-compat issue. A lot of the ecosystem's problems tend to get blamed on TS, when it's actually TS playing along with the ecosystem's desire to be custom at every turn.

[SimenB]

Ah right. Fair enough 🙂

[sibelius]

related #15354