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

Edge support #119

Closed
alexandercerutti opened this issue Apr 5, 2019 · 11 comments

Comments

@alexandercerutti
Copy link

commented Apr 5, 2019

Hello everybody!
I'm using Ky on an important project at work. Sadly I have to support also Edge but it seems Ky does not support it. Is there a specific reason? Will it be supported? Or at least, will the TRIDENT version of Edge - until Microsoft will release Chromium-based Edge?

Thank you very much.

@sindresorhus

This comment has been minimized.

Copy link
Owner

commented Apr 5, 2019

It's in the readme:

For older browsers, you will need to transpile and use a fetch polyfill.

"Older browsers" meaning Edge too. No plan to do anything else for Edge.

@sholladay

This comment has been minimized.

Copy link
Collaborator

commented Apr 5, 2019

Aside from the issue of us officially supporting it, Edge has all, or nearly all, of the things we depend on, so it should be pretty easy to fix whatever is broken. I'd be happy to at least look into it. Can you provide any details about what's not working?

My first guess is lack of support for one of the globals we use. In particular, I was suspicious that Edge might not yet have AbortController. But according to MDN, it was added in Edge 16. Is it possible that you are on an older version of Edge?

@alexandercerutti

This comment has been minimized.

Copy link
Author

commented Apr 5, 2019

Thanks for the reply @sindresorhus.
That was absolutely not clear, as even if it's not one of the best browser, Edge shouldn't be considered as an older one imho.
Anyway, it was giving me syntax error... I don't know if the polyfill would make any difference...

@sholladay, as I said it gave syntax error on something like response result (an object with rest operator + [key], literally).

I don't remember anything else and I won't be able to see the error until Monday... The fact is that then, on Monday, at work we'll remove Ky to move to axios (which is already a dependency, but I was pushing to move to Ky, as more lightweight and was fitting perfectly with the structure I created).

Oh, p.s. I tested Edge through Virtual Box and Parallels desktop with the "light" version of Windows that are provided freely for 90-days by Microsoft to test Edge and IE.

@sholladay

This comment has been minimized.

Copy link
Collaborator

commented Apr 5, 2019

Did you import Ky as a module or just with a normal <script> tag? Forgetting to import as a module is an easy mistake to make that will lead to a syntax error. You can use the UMD build if you can't or don't want to use Ky as a module for some reason.

@alexandercerutti

This comment has been minimized.

Copy link
Author

commented Apr 5, 2019

No, I'm using it in a Typescript project, which is web-packed along with other projects and run on React+Babel+Typescript.
For this reason, my first idea was to let babel transpile this module, but it didn't work (also, I never used Babel)

@sholladay

This comment has been minimized.

Copy link
Collaborator

commented Apr 5, 2019

Okay I tried this out for myself and indeed, Edge chokes on Ky's syntax. Specifically, the stack trace points to this line as the cause of the syntax error.

That line uses object spread, which apparently isn't supported by Edge yet, according to MDN. Go vote for this issue on their Chakra engine to let them know you want it. When that's released, Ky will hopefully "just work".

Until then, it would indeed be possible to transpile Ky with Babel. You'll have to configure it to do so explicitly, since by default, Babel will ignore things inside of node_modules.

@alexandercerutti

This comment has been minimized.

Copy link
Author

commented Apr 5, 2019

Awesome, thank you very much!
Any clues how to tell it explicitly?

@sholladay

This comment has been minimized.

Copy link
Collaborator

commented Apr 5, 2019

Look at the include and exclude options to tell Babel what to process.

If I remember right, I think I had to tell it to exclude everything in node_modules except for the specific library I wanted to transpile.

@alexandercerutti

This comment has been minimized.

Copy link
Author

commented Apr 6, 2019

I'll try this on Monday, but I'm not sure I'll be able to do this. Is any Babel extension needed?
Also, as I said, we planned to remove Ky, so... I don't know.

@sholladay

This comment has been minimized.

Copy link
Collaborator

commented Apr 6, 2019

Your Babel config probably already includes the necessary plugin, @babel/plugin-proposal-object-rest-spread, but you can always try adding it just in case. The main problem is that Babel is ignoring node_modules, so you need to ensure your config tells Babel to include node_modules/ky/**.js.

This is not s problem with Ky. You will encounter the same problem with any library that uses modern syntax unless your build system is setup to handle it.

@alexandercerutti

This comment has been minimized.

Copy link
Author

commented Apr 8, 2019

@sholladay, I've solved this way:

// Webpack configuration file

module: {
    rules: [{
        test: /\.(js|jsx)$/,
        use: 'babel-loader',
        exclude: /(node_modules(?!\/ky)|bower_components)/,
    }, {
        ...
    }]
}

Thank you very much again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.