Typescript error in "moduleResolution": "node16" due to "type": "module" in package.json

[quickgiant]

Hi, thanks for the great work on this package. I've run into an issue when trying to import it from a project using Typescript 4.7 with compilerOptions.moduleResolution set to "Node16" and the output module format set to "CommonJS". Due to "type": "module" being set in msgpackr's package.json, Typescript treats the main types entry point as being ESM-only, even though based on the main, module, and exports fields, both ESM and CommonJS entries are available. This results in the following error when importing from a CommonJS context:

Module 'msgpackr' cannot be imported using this construct. The specifier only resolves to an ES module, which cannot be imported synchronously. Use dynamic import instead.

The same issue was brought up in the Typescript repo here with some discussion on ways to approach it: microsoft/TypeScript#49299

From what I understand, there are a couple options:

1. Separate the Typescript entry points into index.d.mts and index.d.cts files and extend the "exports" entries to also include "types" conditions, for example:

".": {
  "node": {
    "require": "./dist/node.cjs",
    "import": "./node-index.js"
  },
  "bun": {
    "require": "./dist/node.cjs",
    "import": "./node-index.js"
  },
  "types": {
    "require": "./index.d.cts",
    "import": "./index.d.mts"
  },
  "default": "./index.js"
}

Unfortunately, due to Typescript's interpretation of "type": "module" being so strict, this breaks all of the relative imports inside index.d.cts to other *.d.ts files. Not sure if there's a way around this other than making *.d.cts copies of all the other definitions too, which is pretty gross.

2. Remove the "type": "module" field, and rename all of the *.js files to *.mjs. In this case, Typescript will treat the type definitions as CommonJS, but this probably doesn't matter since the major difference between the two modes from Typescript's perspective seems to be how default exports are handled, of which msgpackr has none. This option is less correct, but probably more compatible.

Here is a minimal repro:

package.json

{
  "name": "msgpackr-repro",
  "version": "1.0.0",
  "main": "dist/test.js",
  "scripts": {
    "build": "tsc"
  },
  "dependencies": {
    "msgpackr": "^1.6.2",
    "typescript": "^4.7.4"
  }
}

tsconfig.json

{
  "compilerOptions": {
    "module": "CommonJS",
    "moduleResolution": "Node16",
    "outDir": "dist"
  },
  "files": ["test.ts"]
}

test.ts

import { pack } from 'msgpackr';

pack({ foo: "bar" });

[kriszyp]

Thank you for the detailed report, this is very helpful!
In my testing, it seemed that this commit was sufficient to properly compile your reproduction package:
b372fe7
Do you think this should work, or do you see pitfalls with this change?

[quickgiant]

Changing the index.d.ts to index.d.mts does get rid of the error in my repro, but only because it prevents TS from resolving the package at all under CommonJS, resulting in an implicit any. If you add "noImplicitAny": true to the repro tsconfig.json, we get an error that it can't find a declaration file.

Another alternative similar to option 2 would be to keep "type": "module" for the runtime code, but to have the types be *.d.cts instead of *.d.ts. I don't think Typescript can be configured to emit .cts files from regular .ts files or from .mts files, so they would need to manually be renamed before publishing.

The takeaway is that if the package is supposed to be consumable as CommonJS with type definitions, there needs to be some way for Typescript to import the definitions themselves as CommonJS in order for them to work under "moduleResolution": "node16". It's kind of a mess honestly, and a lot of packages have similar issues.

The one saving grace here is that, unlike CJS importing ESM, ESM is pretty permissive about importing CommonJS, so as long as the types behave the same in both contexts, you can mostly get away with it.

[kriszyp]

Here is another attempt:
#76