ember-fetch

ember-fetch copied to clipboard

It's a best practice in general, for imports to come from their package name rather than some more "convenient" name, and will become more so as Embroider lands:

New addons should not use renaming rules because it's confusing when the imports people type don't align with their real dependencies.

The import style here should simply be:

import fetch from 'ember-fetch';

This is not a blocker for use with Embroider, as there are solutions to support the existing flow, but is a nice improvement and will eliminate one thing that has to be done for this to use Embroider. It also has the nice upside of eliminating all shenanigans required to have this do the right thing for TypeScript integration!

This would be a breaking change, but should also be trivially codemod-able.

We could start by providing both modules, and deprecate the fetch one. Wouldn't need to be a breaking change until we drop support (which could happen "far in the future").

Yeah, I like that as a path forward.

Given that global fetch is a stable WHATWG standard, what do we gain by keeping an importable ember-specific package, as opposed to polyfilling the global fetch in each environment we care about (just fastboot and IE11, I think) and declaring that all Ember apps and addons are free to use global fetch?

Implementation-wise, the two are equally interceptable.

Seems perfectly reasonable to me. We’d just want a nice configurable switch for that if it’s ending up in the vendor bundle, or (better?) to deliver it separately.

I think thats fine in general, it would also allow easier "use native" (since we would literally do nothing if your targets all have fetch).

We'd have to confirm that its "fine" in SSR environments too (since there is not a fetch global there), it would add to the list of expected globals in the fastboot sandbox setup (which is probably fine, but worth mentioning).

Given that global fetch is a stable WHATWG standard, what do we gain by keeping an importable ember-specific package, as opposed to polyfilling the global fetch in each environment we care about (just fastboot and IE11, I think) and declaring that all Ember apps and addons are free to use global fetch?

@ef4 Im perfectly good with this, I prefer that approach.

Questions:

• In this world, does ember-fetch provide said polyfill or something else
• Currently import fetch from 'fetch'; is always the polyfill, to ensure that 100% compatibility between ie11. This is clearly not ideal, but the alternatives come with some risks. My thought was, once i11 is off our radar we re-eval
• Currently the polyfill version also provides support for libraries such as pretender, which tap into XMLHttpRequest via FakeXMLHttpRequest(which is what the polyfil uses). This was done out of pragmatism, and I suspect alternative approaches could be taken.

Forcing the use of import fetch from 'fetch'; prevents Ember apps from using other npm packages that rely on the global fetch API, along with Response, Request, Headers, etc..

This isn't just an issue for IE11, where ember-fetch doesn't polyfill the fetch global..

It's also an issue for libraries that expect the native Response object. For example..

import { Response } `fetch`;

console.log(Response) // Response() { [polyfilled code] }
console.log(window.Response) // Response() { [native code] }
console.log(Response instanceof window.Response) // false

Another example is Rollbar, which tracks events for Telemetry, in part by wrapping the global xhr and fetch APIs

Following up on this (as I'm poking at some related things):

• If we want a "ponyfill" that supplies native if it exists or a polyfill if not, that's fine, but it should be from ember-fetch instead of fetch as the module import, for the reasons articulated a couple years ago.
• But also: this whole package is mostly irrelevant in a post-🔥 DIAF IE11 🔥 world, and should probably just be put into "maintenance/critical bug fixes only" mode once Ember 4 comes out.

ember-fetch also polyfills AbortController which isn't available on Safari until 12.1. https://caniuse.com/?search=abortcontroller the support has good coverage now.

IMO in a post (IE11 / Safari12) world, we should just move the fastboot support part into fastboot its own (maybe think about how we expose the isomorphic fetch API a bit more), and not rely on polyfill in the browser at all.

Node 18 is likely to add fetch.

it did :tada:

https://nodejs.org/dist/latest-v18.x/docs/api/globals.html