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

No ES5 package #28

Closed
jsardev opened this issue Sep 12, 2018 · 6 comments
Closed

No ES5 package #28

jsardev opened this issue Sep 12, 2018 · 6 comments

Comments

@jsardev
Copy link

jsardev commented Sep 12, 2018

This library doesn't work with popular tools like create-react-app or next.js. The reason is that it has no precompiled ES5 bundles. This is considered a bad practice as when someone is using it he has to compile it to ES5 himself to support older browsers. Also, ky won't work on older browsers when just put into the page as a script.

I don't think it's a good idea to force users to compile every dependency to make sure it's ES5-ready. It would massively affect build times.

@sindresorhus
Copy link
Owner

I don't transpile my modules. If you need support for older browsers, it's up to you to transpile for what you target. sindresorhus/ama#446

FYI, create-react-app v2 will transpile dependencies, and will hopefully be out soon.

@jsardev
Copy link
Author

jsardev commented Sep 12, 2018

I agree that this issue should be considered in tools like webpack or babel, but I guess it's not coming anytime soon. And for the sake of simplicity of using the library, I'll still definitely go for a transpile, at least for the current standard (which is ES5 for now), as this is ridiculously easy with tools like rollup or parcel. That would save a lot of time of developers finding out why this is not working (because the errors are not very descriptive) - you can see clearly how much on the issue you've just linked. But... that's just my opinion, you're still the boss here 😄 Thanks for explaining your position on this!

P.S. Of course I mean only packages that are specifically aimed for browsers. Like this one 😄

@matej-prokop
Copy link

matej-prokop commented Sep 20, 2018

I like idea behind this library. Unfortunately, I am not able to use it as other thousands of developers using create-react-app.

Reply to #28 (comment):

@sindresorhus Please consider to build version compatible with current version create-react-app. I am sure that it will help KY adoption a lot. If you check download rate at https://www.npmjs.com/package/ky and compare it to number of GitHub stars then you have to see unbalance there. I believe it is because ppl like KY concept but they can't use it.

@ianwalter
Copy link
Contributor

In order to get this to work I would normally just add the module (ky) to the Webpack babel-loader include like so:

      {
        test: /\.js$/,
        include: [
          path.join(__dirname, './src'),
          path.join(__dirname, './node_modules/ky')
        ],
        loader: 'babel-loader'
      },

But it seems that with the new version of @babel they no longer allow you to use the root project's babel config (in .babelrc or package.json) to transpile another module with it's own package.json so I get an error that says Support for the experimental syntax 'objectRestSpread' isn't currently enabled even though my root project's config is using a preset that DOES have support for this.

So for anyone else running into this, the only solution I've found is to move the config out of package.json and into babel.config.js.

@matej-prokop
Copy link

I don't believe that stable CRA v2 will be release in next 6 months - I would love to be wrong here ;)

LOL, create-react-app v2 was just released 🤣(see https://reactjs.org/blog/2018/10/01/create-react-app-v2.html)

@sindresorhus
Copy link
Owner

Continued in #44

Repository owner locked and limited conversation to collaborators Oct 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants