Ky: Does not work when mocking `window.fetch`

[luxferresum]

This is very bad for testing.
when using ky tools like pretender can not work, because ky stores a reference to fetch immediatly after its loaded with const fetch = getGlobal('fetch');.

This means when a tool like pretender overwrites window.fetch for testing ky does not use the new window.fetch but the stored reference to the real fetch and so does a real request.

Is there a good reason to not call getGlobal('fetch') whenever fetch is required? (currently only here and here)

I'm willing to do a PR, but only if it will be merged.

[szmarczak]

I'm willing to do a PR, but only if it will be merged.

You can do a PR only if you want to. Nobody says you have to. That would be cool. Rejected PRs don't make you look bad. The opposite, they make you look better because you showed some effort.

I mean it would be cool to see this in production.

[sholladay]

We swap out the fetch implementation just fine in ky-universal and in our tests. You simply need to overwrite window.fetch before ky is loaded. That seems like a reasonable requirement to me, because it avoids any race condition issues or other bugs caused by the order in which you mock fetch and use ky. You have to get the environment ready ahead of time.

[szmarczak]

@sholladay What about putting these:

https://github.com/sindresorhus/ky/blob/2ea165a60614729c0d1fe79070541e8bec09575b/index.js#L24-L30

into an object and exposing it via ky.globals?

[sholladay]

I think I'd rather put the getGlobal() calls inline, as suggested above, instead of having these globals as part of our public API surface, personally.

I'm curious to know why mocking fetch before Ky loads is not enough. It's definitely a safer way to do things.

[sindresorhus]

I agree with @sholladay. The convention is to mock before importing something.

[szmarczak]

That's one way to go. If we were using ky.globals, we wouldn't have to do

https://github.com/sindresorhus/ky-universal/blob/ddb2a6417724e7b3cf994061aa688962616b5bd5/index.js#L6-L28

but just pass an object with these values. Although I don't see anything wrong with the current solution.
I'll leave it up to you guys, I'm good with both solutions 😃

[luxferresum]

We swap out the fetch implementation just fine in ky-universal and in our tests.

Thats not true. The tests of ky actually spin up a test server.

I agree with @sholladay. The convention is to mock before importing something.

I highly disagree!

This makes it almost impossible to write independent tests with tools like mocha qunit or testem. Users expect tools like pretender to just work there. This is very important when running browser-only tests.

So people want to do things like this:

let server;
hooks.beforeEach(function() {
  server = new Pretender();
});
hooks.afterEach(function() {
  server.shutdown();
});

---

some context:
I'm trying to integrate ky into ember-fetch-service. This should be a replacement for ember-ajax which uses jQuery. The ember community currently uses a lot ember-ajax and ember-fetch (which is basically just the fetch mock). However this lacks important features. So I have to think about two kind of tests:

1. my own testsuite for ember-fetch-service
2. tests by users of ember-fetch-service

The later is especially problematic. I don't think its a good idea that all apps should need to hook into the test loader (which loads the app and test tests, so I think its the only place to import something before the app is loaded, which then always will import ky, because thats the way import works) to test a single component using ky with ember-fetch-service.
Also this means we can not just teardown pretender and recreate it to reset the state.