Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[Proposal] Set target to ES5 #19

Closed
davidgomes opened this issue Feb 2, 2021 · 10 comments
Closed

[Proposal] Set target to ES5 #19

davidgomes opened this issue Feb 2, 2021 · 10 comments

Comments

@davidgomes
Copy link

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
Copy link
Owner

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
Copy link
Author

  1. IE10/IE11 and maybe even Chrome <48.
  2. https://github.com/adaltas/node-csv-stringify (https://github.com/adaltas/node-csv-stringify/tree/master/lib/es5)
  3. 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
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

Makes sense. Thanks.

@callumlocke

This comment has been minimized.

@Luccasoli

This comment has been minimized.

@astoilkov
Copy link
Owner

@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
Copy link
Owner

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
Copy link
Owner

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants