Problems with Node.js `--experimental-detect-module`

[andrewbranch]

Node.js’s --experimental-detect-module flag poses a problem for TypeScript. If a future release of Node.js enables it by default, there is little we can do to model its semantics in our own module system under --module nodenext, which could lead to a confusing developer experience for TypeScript and JavaScript authors relying on tsc or editor language features in projects configured for Node.js.

This is fundamentally a TypeScript problem, and I don’t know that the severity merits lobbying Node.js to change course. However, I still thought it was important to document and share as feedback.

The problem

1. When TypeScript reads a type declaration file, it needs to know the true module format of the JavaScript file it represents.
2. The contents of a declaration file do not always provide a reliable indication of the true module format of the corresponding JavaScript file.

When a user compiles a TypeScript file like

export default 0;

they can vary the module format of the output JavaScript file by changing the --module flag, but the output declaration file always looks like

declare const _default: 0;
export default _default;

So, that declaration file text on its own is ambiguous. It could represent either of these JavaScript files:

export default 0;

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

or scripts in AMD or SystemJS format.

For TypeScript users who want to run their code in Node.js, it’s important that TypeScript can distinguish between these two cases, because it affects whether those users can access that module with certain types of imports, and what type should be assigned to the names imported from that module:

import z1 = require("export-default-zero"); // Error if resolved file is ESM

import z2 from "export-default-zero";
z2.default; // 0 if resolved file is CJS
            // undefined if resolved file is ESM

Currently, Node.js’s own module format detection algorithm resolves this ambiguity for us. The file extension of the declaration file tells us the file extension of its JavaScript counterpart, which tells us how Node.js will interpret that file’s module format:

Declaration extension	JavaScript extension	Module format
.d.mts	.mjs	ESM
.d.cts	.cjs	CJS
.d.ts	.js	Determined by package.json

These constraints allow us to make a safe assumption about the format of every JavaScript file represented by a declaration file. For example, when we see a .d.ts file, we know it represents a .js file whose format is determined by its package.json scope—if we find that package.json and see that it does not include a "type" field, we assume the JavaScript file contains CommonJS syntax. If that assumption is wrong, the user’s problem is that their JavaScript file is misconfigured for usage in Node.js—they don’t have a problem with TypeScript.

--experimental-detect-module prevents us from being able to make safe assumptions about the module format of .js files whose package.json scope does not define a "type". The flag makes Node.js detect the module format of those files by reading their contents, but TypeScript can’t do the same in the parallel world of declaration files due to the syntax ambiguity discussed earlier.

Possible mitigations

Resolve and read JavaScript files when declaration files are ambiguous

TypeScript avoids reading, or even checking for the existence of, JavaScript files when declaration files are found first. The declaration files currently tell the compiler everything it needs to know about the presence and contents of JavaScript files, so there’s no point in spending extra memory and file I/O reading JavaScript files. If that changes, we could potentially resolve ambiguity by looking at JavaScript content. This would come with a performance penalty. Additonally, multiple team members expressed discomfort at the prospect of giving up the invariant of declaration files being ultimate sources of truth for the compiler. As things stand now, this is the mitigation we’re least likely to pursue.

Eliminate declaration file ambiguity going forward

We could begin changing our declaration file emit format to ensure that module formats are not ambiguous in the future. This is something I strongly advocate for, but it isn’t a solution on its own, because so many existing packages are already ambiguous, and many will never update.

Assume ambiguous files are CommonJS (i.e., do nothing)

If Node.js made --experimental-detect-module the default in the future, we would advocate strongly that all packages define a "type" in their root package.json to avoid ambiguity. (We would likely consider mandating this when compiling local projects under --module nodenext.) We could rely on this becoming best practice for new and actively maintained packages, and assume that packages lacking a "type" field were published before --experimental-detect-module, when such a lack of a "type" implied CommonJS.

Type imports of ambiguous files as permissively as possible

It might be possible to do some type system trickery to make something like this work:

import z1 from "export-default-zero";
z1;         // Type: AmbiguousModule<0> ~= 0 & { default: 0 }
z1.default; // Type: 0

This would be an attempt to get out of the way and never issue a false positive error, at the expense of missing some real errors.

Allow users to assert the format of ambiguous imports

In combination with some other behavior as the default, we could allow users to resolve the ambiguity themselves, perhaps with an import attribute or some central configuration.

[aduh95]

we would advocate strongly that all packages define a "type" in their root package.json to avoid ambiguity. (We would likely consider mandating this when compiling local projects under --module nodenext.

I'd in favor of that, even if we end up using one of the other options to workaround that issue, or if Node.js never makes --experimental-detect-module the default; pushing for less ambiguity is a win for the whole ecosystem IMHO.

[GeoffreyBooth]

we would advocate strongly that all packages define a "type" in their root package.json to avoid ambiguity.

Node also encourages everyone to always include a "type" field, especially for published packages: https://nodejs.org/api/packages.html#determining-module-system:~:text=For%20this%20reason,should%20be%20interpreted. Perhaps tsc could sometimes error when "type" is missing, such as when run in a version of Node where detect-module is enabled by default or when certain settings are specified.

[andrewbranch]

That’s something we might consider for a user’s project package.json, but the real issue for us is the thousands of existing npm packages that have no "type" and will never be updated to get one. We can’t reasonably error on those, but we still need to know what format their .js files are.

[karlhorky]

As Node.js tries to move forward and navigate the migration to ESM, wonder if any large-scale ecosystem projects would help.

First ideas just off the top of my head to motivate conversation, of course each idea with downsides / ownership concerns:

1) Publish package details

1a) npm does a 1-time analysis of all 2.6 million packages to find out the module format, using the same heuristics Node.js does with the --experimental-detect-module flag
1b) npm provides a location where these details will be published (eg. returning the data in the existing API / registry routes or providing a completely separate location)
1c) npm performs this analysis + publishing on every publish of a module without the "type" field in package.json
1d) npm provides deprecation warnings for packages published without "type" in package.json
1e) npm fails to allow publishing of modules without "type" in package.json (after deprecation period)

1a-1c could also be taken over by a different organization eg. DefinitelyTyped / Microsoft in case the value to npm wouldn't be high enough.

2) Serve "type": "commonjs"

2a) npm serves all package.json files which are published without "type" as package.json files with "type": "commonjs"

This one feels more controversial and complicated, altering the files returned by a package. I'm sure there are a number of storage, caching and consistency concerns here (maybe also security concerns).

---

I don't expect these to be taken into consideration wholesale without deeper investigation and proper planning. I just mean to open the discussion that potentially some large-scale work would make the transition to ESM easier.

cc @MylesBorins @lukekarrys

[GeoffreyBooth]

2a) npm serves all package.json files which are published without "type" as package.json files with "type": "commonjs"

Node asked npm about this when --experimental-default-type was being designed, and they said that it’s a hard requirement that files aren’t modified by npm once they’re published, for security reasons. That’s why --experimental-default-type doesn’t apply to folders below node_modules, because all typeless packages would be broken under --experimental-default-type=module and therefore the flag would be unusable unless users did some kind of post-install transform of all their dependencies to add the missing "type": "commonjs" entries.

--experimental-detect-module doesn’t have this issue, as code that can run as CommonJS will run as CommonJS, and so it doesn’t have a node_modules exception. It’s also preferable to have code behave the same way whether or not it’s under node_modules, so that the library author and the library consumer have code run the same way (at least when they’re both using the same version of Node).

[GeoffreyBooth]

TypeScript avoids reading, or even checking for the existence of, JavaScript files when declaration files are found first.

For what it’s worth, within Node I implemented a containsModuleSyntax function in C++ to answer this one question very quickly. We could expose this as a public API if TypeScript might find it useful. It could be exposed before --experimental-detect-module is enabled by default, so therefore TypeScript could include logic like “if the Node version is < the version where module detection is enabled by default, assume CommonJS (prior behavior); else read the file and use Node’s containModuleSyntax to see how Node would interpret it.” The new API would exist in any version of Node where TypeScript would need it, because of the ability to assume the current behavior for prior versions.

Yes this would be a performance penalty, just like detection itself is a performance penalty, which is why we encourage everyone (especially package authors) to include "type" fields. (Or potentially even add missing "type" fields to dependencies in a post-install pass.) But it would be the most direct solution, at least until packages are updated with "type" fields and/or unambiguous type definition files.

[andrewbranch]

I experimented with ways to mitigate the performance penalty of reading and analyzing additional JavaScript files. We have two limitations that take all the good options off the table: (1) our API is synchronous, so parallelizing the work within the same process is not practical, and (2) file I/O is delegated through a host object provided via the JavaScript API (this is what lets us read from unsaved editor files in TS Server, for example), so calling into another process (whether native code or parallelized JS) that performs file system operations is not possible.

[guybedford]

@andrewbranch I know you likely don't want to be implying Node.js directions here, but I'd be interested to hear if you feel this motivates Node.js not unflagging this feature in future?

[andrewbranch]

It’s complicated. I keep waffling between two plausible perspectives.

On one hand, if we think about think about the scenarios that are most likely to be problems for TypeScript in an unflagged future, those are scenarios that are currently problems for Node.js and will be fixed by unflagging this behavior—loading “unmarked” ESM from packages that included it for bundlers, for example. Meanwhile, TypeScript’s declaration files being ambiguous is a problem of TypeScript’s own creation. Node.js is doing right by their users, and it’s on TypeScript to solve their side of things.

On the other hand, given my present feedback that TypeScript won’t be able to solve our side of things to the degree that we would hope, Node.js probably needs to adjust their expectations for how much unflagging --experimental-detect-module is going to improve the end user experience. If Node.js Just Works™ in more cases than it used to, but most of those cases retain red squigglies in VS Code, that’s not a very satisfying improvement for a huge portion of Node.js users. The Node.js team might appreciate knowing that the problem lies squarely with TypeScript, but users are less interested in watching Node.js and TypeScript perform spiderman-pointing-at-spiderman.png than having an experience that works end to end, and I wonder if incrementally changing interop rules, even if in a nominally desirable direction, creates more confusion than it resolves.

I think a good analysis here would require a better understanding of the most common consequences for users. I’ve laid out a theory of how things could go wrong, but I’m not yet sure how often we would see problems and how bad they would be. I’m curious what other feedback about this Node.js has received, and whether they are still leaning towards unflagging, and on what kind of timeline?

I should also point out that because our issues center around declaration files, node_modules packages are a much bigger problem for us than local project files, and especially the loose .js file scenario. I don’t know if it would be on the table to reconsider disabling this behavior inside node_modules, but that would largely eliminate my concerns.

[GeoffreyBooth]

I’m curious what other feedback about this Node.js has received

We haven’t received much. There’s really only one open issue, nodejs/node#50917, which involves the edge case of non-ESM-related parse errors in a file. The “try again as ESM” behavior is triggered by an ESM-related parse error, but if V8 errors on something else before getting to the first import or export statement, the retry never happens; and some of these parse errors are valid when parsed as ESM. The solution is to either just document this, that this is another edge case that won’t trigger running as ESM (like how we currently document top-level await as not triggering detection) or we could remove the limitation of “only ESM parse errors” and try again for any parse error. Either way, though, the solution would be simple.

Once that’s resolved, unless anything else comes up then it’s really just a question of how much more time we want to wait before considering this having been out in the wild long enough to unflag it. Optimistically the earliest that could happen would be in the next few months, sometime near the end of the 21.x line so that we get maximum feedback before 22.0.0 in April. Or we could unflag it as part of 22.0.0, though it doesn’t necessarily need to wait until then as it’s not a semver-major change. And of course it could always happen later, in the 22.x line or beyond, especially if issues are reported.

I think the fundamental thing here is that Node has accepted (or will be accepting, if we unflag this) a conditional performance hit in the name of usability. TypeScript could do the same: that’s the first option in the above list of potential solutions, where tsc reads the accompanying JavaScript file. If Node and TypeScript were managed together as one project, that would presumably be a decision made together; that if Node behaves one way, TypeScript would behave the same. I am very sympathetic to the desire to not slow down any users, but there are many ways to both match Node’s behavior and mitigate or eliminate the performance impact on TypeScript’s side; for example, some new tsconfig option like "dependencyTypeDefinitionsModuleFormat": "commonjs" / "module" / "detect", where the default is "detect" (performance hit, but matches Node) and users could set "commonjs" to preserve current behavior and current performance. I’m not sure it’s worth creating the potential footgun on the Node side of a node_modules exception for this feature, thereby limiting everyone, when TypeScript has the ability to create such an exception just for itself.

[fatcerberus]

watching Node.js and TypeScript perform spiderman-pointing-at-spiderman.png

It’s honestly amazing how often this meme is relevant to the dev scene in general, especially JS. Tech stacks become so intertwined that everyone starts looking at what everyone else is doing for direction, and then before you know it, bam, Spider-Man meme happens.

[guybedford]

I don’t know if it would be on the table to reconsider disabling this behavior inside node_modules, but that would largely eliminate my concerns.

I don't think this would be an option unfortunately, since having packages work locally then break on publish and install would be a really bad footgun.

It's plausible we could unflag in 22 as @GeoffreyBooth describes. So if you do have any strong arguments against that now would be the time to mention I think.

[ljharb]

"not unflagging" still seems like the best option here; the usability isn't worth the risks.