Skip to content

fix(build): Enable compatibility with moduleResolution = nodenext #106

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Feb 20, 2025

Conversation

donmccurdy
Copy link
Member

@donmccurdy donmccurdy commented Feb 19, 2025

Customers using TypeScript configured to moduleResolution="nodenext" or moduleResolution="node2016" will run into the problems described at:

With our current build system (and most others today) type definitions in the build/ outputs contain one .d.ts file per source file, and these files must refer to one another by .js extensions when consumed by any TypeScript project using moduleResolution="nodenext". This could be avoided if we had a build system that bundled all type definitions into a single .d.ts file — in which case there are no imports in the generated types — but I'm not confident recommending a build system with this feature.

TypeScript does not rename extensions from ./foo or ./foo.ts to ./foo.js by itself (microsoft/TypeScript#49462).

deck.gl has a special build step configured to do this renaming.

I'd rather not complicate the build process, so I think the simplest and safest solution is to ensure that all relative imports are .js in the source files, so they'll already be correct when type definitions are built. By adopting moduleResolution="nodenext" for this library we can be sure that the extensions will be consistent, because yarn lint enforces it. For examples and tests it doesn't particularly matter either way.

@donmccurdy donmccurdy requested a review from a team February 19, 2025 21:58
Copy link
Contributor

@felixpalmer felixpalmer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My preference would be to copy what deck does, at it is quite messy to have those '.js' extensions everywhere and I wonder if IDEs will get confused. But OK to go with this solution for now

@donmccurdy
Copy link
Member Author

As weird as it looks, this does seem to be what TypeScript recommends:

Because node16 and nodenext are the only module options that reflect the complexities of Node.js’s dual module system, they are the only correct module options for all apps and libraries that are intended to run in Node.js v12 or later, whether they use ES modules or not.

But ... I do not like TypeScript's choices around bundling at all, so pretty open to changing this. The only other way I've handled this before is rollup-plugin-dts, which worked fine but would mean a second build step. Have heard good things about tsup, which appears to handle this out of the box, but I haven't used tsup yet personally.

@donmccurdy donmccurdy merged commit 8bbd8c5 into main Feb 20, 2025
4 checks passed
@donmccurdy donmccurdy deleted the fix/module-resolution-nodenext branch February 20, 2025 15:22
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Imports should use explicit .js extensions for moduleResolution=nodenext compatibility
2 participants