🐛 Prevent closure over fetch
#1267
Conversation
Defaulting `fetchFn = fetch` keeps it around in a closure. This change avoids that, allowing `window.fetch` to be mocked/monkey-patched more easily
|
✔️ Deploy Preview for redux-starter-kit-docs ready! 🔨 Explore the source changes: 19a0738 🔍 Inspect the deploy log: https://app.netlify.com/sites/redux-starter-kit-docs/deploys/60e80719356b5700077789d9 😎 Browse the preview: https://deploy-preview-1267--redux-starter-kit-docs.netlify.app |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 19a0738:
|
| * A mini-wrapper that passes arguments straight through to `window.fetch`. | ||
| * Avoids storing `window.fetch` in a closure, in order to permit mocking/monkey-patching. | ||
| */ | ||
| const defaultFetchFn = (...args: Parameters<typeof window.fetch>) => window.fetch(...args); |
There was a problem hiding this comment.
Should we check for window here and warn on it? The docs already mention handling this in an SSR env, but it's the only feedback I have.
There was a problem hiding this comment.
Yeah, that's a good idea. I've put it immediately within the fetchBaseQuery creation call though, rather than when a fetch is attempted. Caught quicker, and spams less
| const defaultFetchFn = (...args: Parameters<typeof window.fetch>) => | ||
| window.fetch(...args) |
There was a problem hiding this comment.
| const defaultFetchFn = (...args: Parameters<typeof window.fetch>) => | |
| window.fetch(...args) | |
| const defaultFetchFn: typeof fetch = (...args) => | |
| fetch(...args) |
Given that we had it without the window reference before, let's just keep it like that.
There was a problem hiding this comment.
No problem, changed as per the above, and tweaked the docstring slightly.
The reason I explicitly changed it previously was that I've experienced a setup where Sinon's useFakeTimers will mock window.setTimeout but not setTimeout. But given that I haven't found the exact reason for that, or how version specific or test setup specific the behaviour is, I can't assume that will occur elsewhere.
Co-authored-by: Lenz Weber <mail@lenzw.de>
|
I'll wait for the tests to pass and then get this in. |
phryneas
changed the title
fetch
Defaulting
fetchFn = fetchkeeps it around in a closure.This change avoids that, allowing
window.fetchto be mocked/monkey-patched more easily
Also updates the lockfile