Typescript: Not compatible with config "moduleResolution": "nodenext"

[rubencosta]

Describe the bug

• types field is missing from package.json exports main (.) export and typescript does not fallback to the top-level types.
• Typescript does not understand default exports from {export defautl as ...} import { default as ... } ... doesn't work as of 4.8.3 microsoft/TypeScript#50690

To Reproduce
Configure typescript with "moduleResolution": "nodenext"

Actual Behavior
Typescript can't resolve the package exports and types

Expected Behavior
Typescript can resolve the package exports and types

Versions

• react-loading-skeleton version: 3.2.0
• Typescript version: >4.8

[srmagura]

Hey @rubencosta, thanks for the issue and the PR! I am just going to respond to both of them here.

I tried reproducing the error you are getting but I couldn't. Here is my test project. It's in the node folder, you can ignore all the other top-level folders.

I ran yarn tsc and there were no compilation errors. Can you point out what I missing?

[rubencosta]

Hi @srmagura, thanks for the quick feedback!

You are correct, I missed that the package type should be set to module. Here's a fork of the test project where the issue is reproducible.

[srmagura]

Thanks for that. I did some more testing on this with "type": "module" and discovered a few things.

"moduleResolution": "bundler"

This is a new module resolution option introduced with TypeScript 5.0 that I think will become the recommended practice for frontend projects. More info in TS 5.0 announcement post

With "moduleResolution": "bundler" in my tsconfig.json, there were no TypeScript errors when compiling the test project against react-loading-skeleton 3.2.1 (what's currently published on npm).

"moduleResolution": "nodenext"

I did manage to get this to work by making a number of tweaks to node_modules/react-loading-skeleton:

1. Set "type": "module" in package.json.
2. Add "exports" to package.json, copy-pasted from your PR.
3. Edited index.d.ts to include the .js extension in each import statement.

Conclusions

Regardless of which of the two moduleResolution options you choose, it seems we can fix the issue without making a breaking change to react-loading-skeleton, like changing the default Skeleton export to a named export.

After some research, I think the correct move is to make the necessary changes to react-loading-skeleton to get it to work with nodenext. I think this is the right choice because it gives us:

• Compatibility with "type": "module" in Node.js when a bundler is not used
• Compatibility with TypeScript's "moduleResolution": "nodenext"
• (???) Compatibility with native browser ES modules

Next Steps

Someone should make a PR to react-loading-skeleton that does the following...

• Sets "type": "module" in package.json.
• Adds "exports" to package.json, copy-pasted from PR Fix typescript module nodenext compatibility #182.
• Sets "moduleResolution": "nodenext" in our TSConfig.
• Tweaks rollup.config.mjs so that our CommonJS bundle has a .cjs file extension.

We will need to test this in a variety of contexts to make sure it does not break anyone.

I don't think any of these changes would be considered breaking, but I am considering a major version bump, just to be cautious.

@rubencosta Are you interested in making these changes? If not, I can do it.

[srmagura]

We will need to test this in a variety of contexts to make sure it does not break anyone.

We can use my react-loading-skeleton-test-projects repository as a starting point for this.

One thing in particular we should test is that these changes don't break compatibility with older TypeScript versions. As of this writing, the oldest version still supported by DefinitelyTyped is 4.3. (link)

@RoseMagura Are you interested in working on this?

[rubencosta]

@srmagura Thanks for the time and very useful info. Here are my 2 cents after a bit more evaluation. I did try your proposed solution and tweaks using both the test repo and my repo.

"moduleResolution": "bundler"

This definitely solves all issues caused by having a package type module and typescript. However, it's not recommended for libraries and my use case is an UI elements library. This would mean that I would have to set "moduleResolution": "bundler" in my lib tsconfig.json and upgrade to TS >5.0 to make this work.

"moduleResolution": "nodenext"

To be honest I could not reproduce the solution you described. The only way I could get ES module to work is by having everything as named exports (what I did in #182) but this is probably due to my lack of knowledge.

Argument for named exports

Since your proposed solution still requires a major bump (which sounds reasonable to me, because even if we don't make breaking changes on the exports the package type is changed) I would still push for making everything named exports as this would make the package exports consistent, avoid rollup warnings and effectively fix the compatibility issues for all ts versions because everything supports named imports or default imports, it's the mix that's causing the issue from what I can see.

[srmagura]

I prefer everything as named exports too, but I don't want to make everyone using this library refactor their imports, if it is possible to avoid.

You mentioned "rollup warnings". Is this something happening in your project? What is the warning?

[rubencosta]

@srmagura I meant these warnings in rollup config.

Without testing any of the following: would it be possible to still expose the default export in parallel with the named export marking the default export as deprecated and documenting that users should move to named exports in future version? Assuming that the named export would fix this issue.