astoilkov / use-local-storage-state

React hook that persists data in localStorage
MIT License
1.17k stars 41 forks source link

Please provide a CJS export. #71

Closed dwjohnston closed 1 week ago

dwjohnston commented 2 weeks ago

Currently (v 19.4.1) the imported package looks like this:

//package.json 
    "type": "module",
    "exports": {
        "types": "./index.d.ts",
        "default": "./index.js"
    },
    "sideEffects": false,
    //snip
    "files": [
        "index.js",
        "index.d.ts",
        "src/**/*.js",
        "src/**/*.d.ts"
    ],
//index.js

import useLocalStorageState from './src/useLocalStorageState.js';
export default useLocalStorageState;

The problem with this is that despite ESM being generally well supported, a lot of tooling still has difficulty with it.

For example, Jest has limited ESM support.

I have a reproduction here demonstrating difficulty using this package with Jest

https://github.com/dwjohnston/localstorage-repro

Relevant other resources:

https://jestjs.io/docs/ecmascript-modules

https://stackoverflow.com/questions/68520619/jest-typescript-with-es-module-in-node-modules-error-must-use-import-to-load-e

Suggested solution

Package json looks like this:

  "exports": {
    ".": {
      "require": "./index.cjs", // CJS
      "import": "./index.mjs", // ESM
      "types": "./index.d.ts" // types
    }
  }
dwjohnston commented 2 weeks ago

Incase anyone needs it - adding this to your jest.config might solve this issue for you:

  transformIgnorePatterns: ['node_modules\\/(?!use-local-storage-state\\/)'],

ie. it's telling jest to ignore all node modules, but not this one - please transpile it.

astoilkov commented 2 weeks ago

This package was supporting CJS before but I removed it — https://github.com/astoilkov/use-local-storage-state/issues/61.

Here's also a related discussion on the topic from a few days ago — https://github.com/astoilkov/p-signal/issues/1.

I had some issue maintaining and fixing stuff around CJS so I decided to drop it. Also, there are sindresorhus, wooorm, and other popular open-source contributors that are advocating for move to ESM and I kinda support it.

For example, sindresorhus modules are ESM only and his packages are downloaded 30B+ times a month. I feel like if this is the case, people are able to successfully make their tooling work.

Here's the link that sindresorhus discusses the ESM only move — Pure ESM package.

Another point to make is this chart — https://x.com/wooorm/status/1829180747070181793 — where dual (ESM and CJS) support is less common than ESM only.

dwjohnston commented 1 week ago

Alright, you might have changed my mind here.

The main time I encounter this, is with Jest, but the fix for it is easy enough.

(Well, we'll see, maybe I should retain a list of various toolings that don't behave nicely with ESM).

astoilkov commented 1 week ago

Thanks for thinking about this and continuing with the discussion!

What do you want us to do here? Are you ok with closing this issue and the associated PR?

dwjohnston commented 1 week ago

Sure, let's close it. What I'm going to do is maintain my own list for my own reference of scenarios where untranspiled ESM in packages causes issues.

dwjohnston commented 1 week ago

For posterity - here's the list: https://blacksheepcode.com/posts/list_of_tooling_that_does_not_play_nicely_with_esm :)