Add a compiler error when using `module: esxxxx with moduleResolution: nodexx

[weswigham]

Since this combination of options can have confusing behavior for, eg, .cjs or .cts files, which are emitted as esm in this mode. We don't really intend to support this combination of flags, so while it does work and does do something, it's probably not what whoever combined the flags is hoping for, given what moduleResolution and module are each responsible for. (The other old module flags make a degree of sense, since they trigger emitting everything as cjs/amd/umd/system, which are all downlevel formats)

Fixes #50647

[andrewbranch]

I think we should discuss making the other module modes an error too, or at least commonjs. To emit a .mjs file as CJS, while pretending during module resolution that it’s going to resolve like an ESM file, seems to be an equally broken combination as emitting ESM into a .cjs.

[weswigham]

Mmm, I mean, it sounds kinda bad, but, conversely, when you think about it without the file extension, it's what people do all the time - downleveling esm input to cjs output. It's just that output file extension is kinda eck, since you'd have a .mjs file with a commonjs module inside. But, like, ignoring the "type: module" field and transforming esm .js files to commonjs is exactly why people combine those flags.

[gabritto]

The code looks fine, but as per the comments above, I'd like to better understand a few things. @weswigham @andrewbranch what types of bad things can happen e.g. with moduleResolution: node16 and module: esnext, and what bad things can happen with moduleResolution: node16 and module: commonjs? I get that it can be confusing because module resolution thinks a module is going to be cjs/mjs when it's in fact not, but I could use a more concrete example, if possible.
Also, @weswigham your last comment seems to imply that people have valid reasons to use module: commonjs with node16 resolution. Could you elaborate on those reasons? And does this mean there aren't equally valid reasons for the converse, i.e. using module: ESsomething?

[andrewbranch]

I think we should consider a broader set of errors or fixes for trying to do stuff that’s only going to work in nodenext in other modes. #51990 outlines a pretty reasonable expectation—that tsc main.mts emits ESM by default. I marked it as a duplicate of #50647, but this PR doesn’t help with it, because that user is just using defaults, not nodenext. In many ways, it’s much easier to end up with ESM in .cjs files or CJS in .mjs files, with no errors, than it is to get everything right, and that feels pretty broken.

[knightedcodemonkey]

Why even mess with the files module system when either of the new file extensions are used? The compiler should just leave it alone if the extension is .mts, .cts, .mjs, or .cjs. I've brought this up in greater length on #51990 here. Right now any project that uses --module commonjs and .mts files is going to get a build with broken .mjs files.

Here's an example of what could go wrong.

esm.mts

interface ESM {
  esm: boolean;
}
export const esm: ESM = {
  esm: true
}
export type { ESM }

tsconfig.json

{
  "compilerOptions": {
    "target": "ESNext",
    "module": "CommonJS",
    "moduleResolution": "NodeNext",
    "outDir": "dist",
  },
  "include": ["esm.mts"]
}

package.json

{
  "version": "0.0.0",
  "type": "commonjs"
}

Now run tsc and write some script to consume the build output.

script.js

const doImport = async () => {
  await import('./dist/esm.mjs')
}

doImport()

Now run the script:

node script.js

file:///path/to/dist/esm.mjs:2
Object.defineProperty(exports, "__esModule", { value: true });
                      ^

ReferenceError: exports is not defined in ES module scope
    at file:///path/to/dist/esm.mjs:2:23
    at ModuleJob.run (node:internal/modules/esm/module_job:192:25)
    at async DefaultModuleLoader.import (node:internal/modules/esm/loader:228:24)
    at async importModuleDynamicallyWrapper (node:internal/vm/module:428:15)
    at async doImport (/Users/morgan/code/babel-dual-package/script.js:3:3)

Node.js v20.5.0

Errors also arise when starting from an ESM-first project, i.e. changing to "type": "module" in the package.json file. In this scenario you can update the script to not use a dynamic import() (you get the previous error if you leave it alone):

script.js

import { esm } from './dist/esm.mjs'

Running this script produces this unexpected error:

file:///path/to/script.js:1
import { esm } from './dist/esm.mjs'
         ^^^
SyntaxError: The requested module './dist/esm.mjs' does not provide an export named 'esm'
    at ModuleJob._instantiate (node:internal/modules/esm/module_job:122:21)
    at async ModuleJob.run (node:internal/modules/esm/module_job:188:5)
    at async DefaultModuleLoader.import (node:internal/modules/esm/loader:228:24)
    at async loadESM (node:internal/process/esm_loader:40:7)
    at async handleMainPromise (node:internal/modules/run_main:66:12)

Node.js v20.5.0

Ok, well that's because I know it was compiled to use the CJS module system (unexpectedly), so do the import * as approach to invoke the Module Namespace Exotic Object. Changing the script once again:

import * as esm from './dist/esm.mjs'

And running it once again produces another error:

file:///path/to/dist/esm.mjs:2
Object.defineProperty(exports, "__esModule", { value: true });
                      ^

ReferenceError: exports is not defined in ES module scope
This file is being treated as an ES module because it has a '.js' file extension and '/path/to/package.json' contains "type": "module". To treat it as a CommonJS script, rename it to use the '.cjs' file extension.
    at file:///path/to/dist/esm.mjs:2:23
    at ModuleJob.run (node:internal/modules/esm/module_job:192:25)
    at async DefaultModuleLoader.import (node:internal/modules/esm/loader:228:24)
    at async loadESM (node:internal/process/esm_loader:40:7)
    at async handleMainPromise (node:internal/modules/run_main:66:12)

Node.js v20.5.0

None of this is what one would expect when using .mts or .mjs file extensions. Using those extensions is effectively opting in to the ES module system no matter what --module value is passed to the compiler. Adding compiler warnings is not the solution. It also shouldn't be a matter of why one would do that because the fact is one can do that in Node.js, so TypeScript should align with that behavior.

[knightedcodemonkey]

By the way, if TypeScript was thinking people using --module commonjs would want to consume the emitted .mjs file with require, well that is broken too (in the philosophy and implementation).

package.json

"type": "commonjs"

script.js

require('./dist/esm.mjs')

node script.js

node:internal/modules/cjs/loader:1089
    throw new ERR_REQUIRE_ESM(filename, true);
    ^

Error [ERR_REQUIRE_ESM]: require() of ES Module /path/to/dist/esm.mjs not supported.
Instead change the require of /path/to/dist/esm.mjs to a dynamic import() which is available in all CommonJS modules.
    at Object.<anonymous> (/path/to/script.js:1:1) {
  code: 'ERR_REQUIRE_ESM'
}

Node.js v20.5.0

The generated output for .mts extensions is just wrong.

"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
exports.esm = void 0;
exports.esm = {
    esm: true
};

Is there a workaround for this?

[knightedcodemonkey]

Setting --module nodenext will preserve the .mts file's module system, as well as any .cts file. However, there is no way to then take any other .ts file in the same project, and convert it to the CJS module system by using another module resolution value like --moduleResolution node (or even node10).

--module and --moduleResolution are coupled when the value is nodenext.

[andrewbranch]

This can be closed; it was superseded by #54567. I forgot this PR existed when I made that one.

[knightedcodemonkey]

So the shortcomings I brought up here have been addressed in #54567? Namely, there is no way to compile .ts files in a project to CJS while preserving the module system of .mts files in the same project?