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

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.

• Various tools start refusing to publish new packages that don’t have "type" set

We asked npm about this when we were working on --experimental-default-type and the answer was basically no, because the npm registry is for more than just Node stuff, and targets like browsers don’t need a type field. They also won’t ever change an already-published package, say to add the type field, because of security; current consumers of old packages might be checking integrity checksums, and those are never supposed to change.

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).

Actually to amend my previous comment slightly, what if:

1. tsc is updated ASAP to start emitting unambiguous type definition files; and
2. Ambiguous type definition files are assumed to be CommonJS, however if a type error is thrown that could be caused by an ambiguous type definition file, the accompanying JavaScript file is read to resolve the ambiguity.

This would close the edge case at the cost of performance, but only under a condition that would be rare to occur (someone relying on detection enabled by default, and who generated the type definition files using an old version of TypeScript). The vast majority of the time would be either correctly-guessed ambiguous type definition files that are CommonJS, or unambiguous type definition files.

Yes, DefinitelyTyped can be updated in bulk, which helps for a large number of old/stale but still-popular packages, but not for ones that ship their own types. I’m not sure we would want to use DT to add module format info for packages that do ship their own types (much less duplicate those types to DT), but Daniel and I did briefly mention the idea of shipping a Bloom filter encoding a set of popular npm packages that are definitely CommonJS. I forgot to mention it in my OP because it was one of the more out-there ideas. I’m not sure it would be a big improvement over “always assume CommonJS.”

I don’t think this is an issue about this new option, but rather about TS in general. More specifically, supporting for CJS, ESM, TS(CJS), TS(ESM) from a single .js file is very hard.

Specifically we have to do stuff like this in fastify:

module.exports = myfn
module.exports.default = myfn
module.exports.myfn

This needs to be matched by a .d.ts file like this:

namespace myfn {
  export const myfn: MyFn
  export { myfn as default }  
}

export = myfn

With all of this, import myFn from 'myfn' and import { myfn } from 'myfn' would work on all the combinations of CJS+ESM+TS above.

Note that this problem is so significant that multiple modules have to:

const X = require('anothermodule')

const x = new X.default() // because default export is not recognized

An alternative solution for module authoring is to https://github.com/isaacs/tshy and have both a CJS and ESM being built.

I do think the new option makes it worse, mostly because it exposes most developers to the same complex DX we have to do on the module authoring side.

I momentarily forgot about the footgun of a package working during local development and breaking when installed into node_modules. I agree that’s not worth it.

This is something that we might want to revisit. Can you open a separate issue about this feedback?

We should not plan to unflag in 22 before the TypeScript story is sorted.

However, I think simplifying the developer experience for CJS+ESM+TS is something we should strive for, as it’s one of the major DX complaints we are hearing from Node.js users. Let’s collaborate.

@andrewbranch @DanielRosenwasser what behavior would work for you? How do you think we can solve this problem?

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.

@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?

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.

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.