tsconfig module: "CommonJS", moduleResolution: "nodeNext" doesn't work (but we're not sure if it's supposed to)

[CaseyHaralson]

Issue Description

When the tsconfig moduleresolution is set to "nodenext", the @apollo/server can't be imported because: The current file is a CommonJS module whose imports will produce 'require' calls; however, the referenced file is an ECMAScript module and cannot be imported with 'require'.

But, changing the moduleresolution to "node" will make the import work.

Is this an issue with Apollo Server?

Link to Reproduction

https://github.com/CaseyHaralson/apolloserver-bug-7222

Reproduction Steps

npm install
npm run build

Then change the moduleresolution type and run the build again.

Edit: updated repo link to include bug number

[glasser]

We do literally have a smoke test for this as part of our CI (smoke-test/nodenext) but thanks for the reproduction — I'll see what's different between your repro and the smoke test.

[glasser]

So, we test with these set:

• module: "nodeNext" in tsconfig.json
• moduleResolution: "nodeNext" in tsconfig.json
• type: "module" in package.json

If I set module: "nodeNext" in your tsconfig.json and either set type: "module" in your package.json or rename the file to src/app.mts, it appears to work properly.

(If I only set type: "module" or rename to app.mts while leaving it as module: "CommonJS", the tsc execution succeeds but creates CJS-style code in build, which seems like it's probably not what you want.)

Is moduleResolution: "nodeNext" intended to be used when you're not using module: "nodeNext"? The TS docs about it are pretty scattered but it's not super clear to me that module: "CommonJS", moduleResolution: "nodeNext" is a combination that's supposed to work well. What are you trying to accomplish with this combination?

[CaseyHaralson]

I've tried a few times previously to move to type: module in the package.json but that makes everything pretty horrible and leaves too many things to fix. So, don't want to make that change.

Renaming the file to .mts might be an option. I'll need to try that. Though, I'm not sure what sort of effects that will have.

I started with module CommonJS because that seems to be recommended for node serverside packages. I'm using npm packages to split code up and kept having to mess with the package imports when the moduleResolution was Node. I switched it to NodeNext and the package imports started working like I would expect. So that is how I got to both of those two settings.

Example of previous package import: '@packagename/src/some-object'. Then I would have to delete '/src' from the import.
Example of package import with NodeNext: '@packagename/some-object'. So it would just work.

Edit: these package imports are being auto included is VSCode. I'll add some sort of reference in the code, VSCode will suggest an import, and taking the default either gives me an import I need to edit, or it gives me the correct import with NodeNext.

[glasser]

I started with module CommonJS because that seems to be recommended for node serverside packages

FWIW I saw something about this in at least one place in the TypeScript docs that strikes me as outdated (ie, not updated after TS4.7 with the new nodenext options). (In general the TS docs are excellent in that any given piece of text is well-written, but so much recent functionality is largely only documented in their (excellent) release notes posts...)

Anyway I think we are happy to accept changes to try to improve this combination of options but figuring it out ourselves on the core team is probably not going to be a priority, given how every time we try to improve anything relating to the module system it ends up being a huge wild goose chase...

[CaseyHaralson]

Ok, I appreciate you taking a look. Do you want me to close this or keep it open? I'm new to the module system so have no idea how to help at the moment.

[glasser]

We can leave it open; I updated the title

[jfrconley]

module: commonjs and moduleResolution: nodenext is a perfectly valid combination. nodenext just follows modern nodejs dependency resolution (supporting the exports field) and CommonJS is still a supported format in nodejs. The issue is that typescript treats .d.ts files with same rules as js files for esm detection. type: module means that exported .d.ts files are treated as modules and as such cannot be used with a require statement even if a main or require export field is provided. The whole ESM situation is an absolute nightmare, but this particular issue is causing some pain for us since we use the exports field, but still compile to commonjs.

You can fix this by providing a separate .d.cts type export under the exports field https://www.typescriptlang.org/docs/handbook/esm-node.html#packagejson-exports-imports-and-self-referencing

[trevor-scheer]

@jfrconley thanks for some insight! The whole typescript + cjs/esm situation is rough. I won't be able to address the issue myself in any meaningful timeframe, but I'd welcome a PR that did if you'd like to take that on?

[benasher44]

Also check out https://arethetypeswrong.github.io/?p=%40apollo%2Fserver%404.7.1, which shows this issue

[trevor-scheer]

@benasher44 neat, thanks for that. We have a script that explicitly removes types from the dist/cjs directories of our packages since it didn't seem necessary...i.e. in our package.json exports field we can only specify one types per export. Do you know of a configuration that lets us ship correct typings for both?

[benasher44]

@trevor-scheer Yes! @andrewbranch did a great job walking through all the edges in a different project's issue, starting from this issue comment

[Cellule]

I took a stab at fixing the typings straight in the repo here.
See my PR #7614

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
For general questions, we recommend using StackOverflow or our discord server.