[Proposal] Set target to ES5

davidgomes commented 3 years ago

Have you considered changing the build target to ES5 instead of ES6 so that this library works across more browsers?

Alternatively, you can ship 2 different builds (one with target=ES5 and one with target=ES6), I've seen some other libraries do this and then as a client, I could do import useLocalStorageState from "use-local-storage-state/es5".

astoilkov commented 3 years ago

I have considered it but not in-depth. I will do that.

If you have the time I will be happy to know the answer to few questions. This will help me while researching:

What browser do you want support for?
Can you give an example for a library providing both ES6 and ES5 support?
Have you tested use-local-storage-state in the browser you want support for?

Thanks in advance.

davidgomes commented 3 years ago

IE10/IE11 and maybe even Chrome <48.
https://github.com/adaltas/node-csv-stringify (https://github.com/adaltas/node-csv-stringify/tree/master/lib/es5)
We have not, but we have reasons to believe it will not work since I looked at the output of use-local-storage-state and there's some const keywords in there which is syntax that's not supported in older browsers.

astoilkov commented 3 years ago

Thanks a lot. One last question: Which bundler are you using? I am asking because I am wondering if it isn't possible to transform the module to ES5 using babel with the help of your bundler.

I will research the topic soon and let you know. Thanks again.

davidgomes commented 3 years ago

We are using webpack and yes, it is possible to transform the module to ES5 using Babel ourselves, but this would be the only dependency that we use that we'd have to do this for, so on our end we'd prefer to have an ES5 output as well.

astoilkov commented 3 years ago

Makes sense. Thanks.

callumlocke commented 3 years ago

I just came here to create an issue that seems possibly related. I just tried using this module in a standard Next.js app (latest version), importing it with import useLocalStorageState from 'use-local-storage-state' into a TypeScript file (just using Next's built-in TypeScript support), and it causes the following try to run or build it (with the latest Node – v14.15.4):

Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: /Users/callum/code/demo/node_modules/use-local-storage-state/index.js
require() of ES modules is not supported.
require() of /Users/callum/code/demo/node_modules/use-local-storage-state/index.js from /Users/callum/code/demo/.next/server/pages/survey/full/[slug].js is an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which defines all .js files in that package scope as ES modules.
Instead rename index.js to end in .cjs, change the requiring code to use import(), or remove "type": "module" from /Users/callum/code/demo/node_modules/use-local-storage-state/package.json.

    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1080:13)
    at Module.load (internal/modules/cjs/loader.js:928:32)
    at Function.Module._load (internal/modules/cjs/loader.js:769:14)
    at Module.require (internal/modules/cjs/loader.js:952:19)
    at require (internal/modules/cjs/helpers.js:88:18)
    at eval (webpack-internal:///use-local-storage-state:1:18)
    at Object.use-local-storage-state (/Users/callum/code/demo/.next/server/pages/survey/full/[slug].js:627:1)
    at __webpack_require__ (/Users/callum/code/demo/.next/server/pages/survey/full/[slug].js:23:31)
    at eval (webpack-internal:///./components/SurveySection/SurveySection.tsx:5:81)
    at Module../components/SurveySection/SurveySection.tsx (/Users/callum/code/demo/.next/server/pages/survey/full/[slug].js:174:1)
    at __webpack_require__ (/Users/callum/code/demo/.next/server/pages/survey/full/[slug].js:23:31)
    at eval (webpack-internal:///./pages/survey/full/[slug].tsx:11:97)
    at Module../pages/survey/full/[slug].tsx (/Users/callum/code/demo/.next/server/pages/survey/full/[slug].js:493:1)
    at __webpack_require__ (/Users/callum/code/demo/.next/server/pages/survey/full/[slug].js:23:31)
    at /Users/callum/code/demo/.next/server/pages/survey/full/[slug].js:91:18
    at Object.<anonymous> (/Users/callum/code/demo/.next/server/pages/survey/full/[slug].js:94:10) {
  code: 'ERR_REQUIRE_ESM'
}

Is there any special config required to use this module in a Next.js app?

Luccasoli commented 3 years ago

I have this same problem too :( i'm using "use-local-storage-state": "^7.0.0",

I just came here to create an issue that seems possibly related. I just tried using this module in a standard Next.js app (latest version), importing it with import useLocalStorageState from 'use-local-storage-state' into a TypeScript file (just using Next's built-in TypeScript support), and it causes the following try to run or build it (with the latest Node – v14.15.4):

Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: /Users/callum/code/demo/node_modules/use-local-storage-state/index.js
require() of ES modules is not supported.
require() of /Users/callum/code/demo/node_modules/use-local-storage-state/index.js from /Users/callum/code/demo/.next/server/pages/survey/full/[slug].js is an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which defines all .js files in that package scope as ES modules.
Instead rename index.js to end in .cjs, change the requiring code to use import(), or remove "type": "module" from /Users/callum/code/demo/node_modules/use-local-storage-state/package.json.

    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1080:13)
    at Module.load (internal/modules/cjs/loader.js:928:32)
    at Function.Module._load (internal/modules/cjs/loader.js:769:14)
    at Module.require (internal/modules/cjs/loader.js:952:19)
    at require (internal/modules/cjs/helpers.js:88:18)
    at eval (webpack-internal:///use-local-storage-state:1:18)
    at Object.use-local-storage-state (/Users/callum/code/demo/.next/server/pages/survey/full/[slug].js:627:1)
    at __webpack_require__ (/Users/callum/code/demo/.next/server/pages/survey/full/[slug].js:23:31)
    at eval (webpack-internal:///./components/SurveySection/SurveySection.tsx:5:81)
    at Module../components/SurveySection/SurveySection.tsx (/Users/callum/code/demo/.next/server/pages/survey/full/[slug].js:174:1)
    at __webpack_require__ (/Users/callum/code/demo/.next/server/pages/survey/full/[slug].js:23:31)
    at eval (webpack-internal:///./pages/survey/full/[slug].tsx:11:97)
    at Module../pages/survey/full/[slug].tsx (/Users/callum/code/demo/.next/server/pages/survey/full/[slug].js:493:1)
    at __webpack_require__ (/Users/callum/code/demo/.next/server/pages/survey/full/[slug].js:23:31)
    at /Users/callum/code/demo/.next/server/pages/survey/full/[slug].js:91:18
    at Object.<anonymous> (/Users/callum/code/demo/.next/server/pages/survey/full/[slug].js:94:10) {
  code: 'ERR_REQUIRE_ESM'
}

Is there any special config required to use this module in a Next.js app?

astoilkov commented 3 years ago

@callumlocke @Luccasoli This issue is not related to the ES5 target. I created a new issue for that – #20. I fixed the issue in a new 8.0.0 release. You can upgrade now but you should do it manually because the ^7.0.0 constraint doesn't allow it.

astoilkov commented 3 years ago

I thought a lot about this. I decided to not add support for ES5. These are my arguments:

ES5 is an edge case. I don't know how much but I believe there is a very small amount of users that need this. IE takes 0.8 percent of the market share.
It's not a deal-breaker. Users can still use @babel to transpile this module to ES5. This solution will be increasingly more popular as more modules don't support ES5.
The trend I am trying to follow is forward not backward. I am currently shipping two versions of the package: non-ESM and ESM and it's overly complicated. I will be soon shipping only ESM(same as Sindre Sorhus). This will probably get even more users mad but this will push the ecosystem further.
It adds complexity to the codebase. I am obsessed with code complexity and this will add an additional step to the build process and complicate it. I don't like that.

astoilkov commented 3 years ago

@davidgomes If you have any other thoughts I'll be happy to hear them.

For now, I am closing this issue for lack of activity.

astoilkov / use-local-storage-state

[Proposal] Set target to ES5 #19