fix: add "exports" field to package.json and rename ESM bundle to .mjs to fix ESM imports on node.js

[zewish]

Summary

Description of issue

Vite 5.0.0+ now removes the exoports.default to default ESM export proxy it used previously.

Unfortunately this breaks the following import import loadable from '@loadable/component' because the dependency is treated as CommonJS export and vite doesn't proxy the default export for CommonJS files anymore.

There is still a flag to enable the old behaviour, but that flag is due to be removed in Vite 6.0.0+, which would break the default import with this extension

What this fix does

This is a quick and dirty fix duplicates the default export as a named export called loadable, so we can also import import { loadable } from '@loadable/component' which works with Vite 5.0.0+.

More info about the deprecation: https://vitejs.dev/guide/migration#ssr-externalized-modules-value-now-matches-production

Alternative (more laborious, but future proof) solution that can be implemented instead of this

I'm currently open to implement this instead since it's way better to have proper exports in terms of future node.js & vite ESM compatibility.

Here is what needs to be done to future-proof the package:

1. '@loadable/component' ESM build should have the .mjs extension.

   • NOTE 1: all major bundlers already support the .mjs extension without any issue.
   • NOTE 2: node.js also supports this natively when the exports map is in the package.json file (see issue 2 for more info).

2. '@loadable/component' should have exports map is set to properly point to loadable.esm.mjs for ESM and loadable.cjs.js for CommonJS respectively.
   

   Example exports map in package.json:

   {
       // ...
       "main": "dist/loadable.cjs.js",
       "module": "dist/loadable.esm.mjs",
       "exports": {
         ".": {
           "require": "./dist/loadable.cjs.js",
           "import": "./dist/loadable.esm.mjs",
           "default": "./dist/loadable.cjs.js"
         }
       },
       // ..
   }

Test plan

I made a build with the current code and tested that the import { loadable } from '@loadable/component' actually works.
Feel free to point me out in case some tests or documentation needs to be updated also.

[theKashey]

🤔 having loadable being named exported will also trigger eslint complain about performing default import instead of named one. Many teams have this rule as an error, so that would be a breaking change for them 😭

Quite a similar issue was reported to React-Focus-Lock with @wojtekmaj proposing a solution theKashey/react-focus-lock#264 very close to the one in PR description, except the mjs comment.

AFAIK recent dramma with bun caused node to move away from mjs, which was always a little strange idea. Pretty sure it's not required. I hope @wojtekmaj have a few insights to share

[zewish]

Hey @theKashey,

Thanks for the quick reply, I really appreciate it! 😃

I'm not sure what you mean by node moving away from .mjs. Sure, you can use "type": "module" inside the package.json file and just use the .js extension for ESM modules instead of .mjs, but this would mean you'll have to rename your CommonJS bundle files to .cjs in case you want to still support CommonJS require() calls. Keep in mind I'm talking about node.js resolution here, not bundlers - bundlers are more permissive when it comes to using whatever extension you picked. (Source: latest node.js docs).

I think we could call this "Solution 3", since I've given two other options in my first comment.

Or, since this is a React library and 99% of the time it goes through a bundler, getting rid of the CommonJS bundle and adding the "type": "module" would probably also make sense, and I'm pretty sure 99% of the users (including me) will be okay with this change. The remaining 1% who're still forced to use CommonJS because of an older version of node - they would have to upgrade to using ESM modules (e.g. new version of node) or introduce a bundler which would handle ESM modules for them.

So this would be "Solution 4" which completely removes CommonJS support.

For me it doesn't matter which one is picked as long as the issue is resolved before Vite 6.0.0 is released and the deprecated legacy ESM proxy for CommonJS modules is removed.

In case you don't have the time to implement a solution I'm pretty much open to contributing once a solution is picked and agreed on. 👨‍💻

Cheers!

[wojtekmaj]

Thanks for @-ing me :D

Indeed, arethetypeswrong/cli points us to some errors:

exports map proposed by OP:

  "main": "./dist/loadable.cjs.js",
  "module": "./dist/loadable.esm.mjs",
  "exports": {
    ".": {
      "require": "./dist/loadable.cjs.js",
      "import": "./dist/loadable.esm.mjs",
      "default": "./dist/loadable.cjs.js"
    }
  },

makes total sense to me. This kills two birds with one stone, by fixing ESM file extension in CJS-by-default package and adding exports map to ensure all bundlers know what to do.

I've had to do some manual work to verify this:

• Copying definitely-typed typings to ./dist/loadable.cjs.d.ts, ./dist.loadable.d.ts, and so on so that all outputted files have their own types
• Building and packing the package locally
• Running @arethetypeswrong/cli on outputted package.tgz

but I can confirm that after these fixes this looks very good:

[wojtekmaj]

(but yeah, you likely don't need CJS build anymore, although removing it would be a breaking change, and the above seems to be an easy, non-breaking fix)

[zewish]

Hey @wojtekmaj, thanks for jumping in and testing solution number 2!
This is also my preferred solution, since it's non-breaking one and it would make both the bundlers and node.js happy.

@theKashey, what would be your preferred solution, and do we need to sync with somebody else before actually doing any work here?

As I said I've got a bit of free time on my hands until Christmas, so I can support you fixing this issue in case you need a helping hand. 😃

[theKashey]

@wojtekmaj +++, I would really prefer not to do a major bump if possible as it will take years for consumers to catch up. Doing something within version ranges creates some technical debt for library, but also reduced for the end product. I think the end product is more important.
So @zewish - that would be my preferred solution, and my approval is all you need here. And you've got it. And I need a helping hand 😅

[zewish]

@theKashey, I think it should as expected now when it comes to importing from ESM modules.

I added a new commit to this PR, which fixes imports and the extension (reverting the previous named export change). Afterwards I ran all the tests - all seems to be good.

I also tested the built bundle with Vite 5.0.0 with the legacy ESM proxy disabled and everything seems to be working as expected.

[theKashey]

Will release tomorrow morning

[theKashey]

Released as 5.16.2