Uh oh!
There was an error while loading. Please reload this page.
Don't polyfill fetch for Node - #5132
Conversation
We used to always import it. Does that cause a problem? Is it broken in Node anyway? In that case I'd be fine with not supporting it there. On the other hand, shouldn't we expect jsdom to add it at some point? In that case it would make sense to polyfill it in Node. At least when jsdom is on. |
Timer
commented
Sep 27, 2018
It's currently broken in Node. In v1, the polyfills were a webpack entrypoint not a Jest setup file, so this didn't bite us until now. |
FYI |
Last universal published 2 years ago, 3 for isomorphic. What about |
Timer
commented
Sep 27, 2018
Hmm, |
gaearon
commented
Sep 27, 2018
OK |
gaearon
commented
Sep 27, 2018
Wait. Polyfills were an entry point in Jest. How else could we get rAF there? |
Timer
commented
Sep 27, 2018
Oh, you're right... 🤔 I'll dig more into this, maybe it never worked and that many people left on the jsdom environment? |
No idea. My high level feedback is: if it was broken before I'd just remove it. If it kinda worked before, I'd leave it in the same state as it was. It should always be removed in non-jsdom environment. I probably wouldn't add something like universal-fetch without more people asking for it. In tests you don't really want fetching to work. You usually want to mock it. |
Timer
commented
Sep 27, 2018
Well, right now Jest can't even load when using the Node env and this at least allows tests to run again. I assume people must've mocked out fetch before us trying to import it, but I think this is a better default. |
Ah, found it. |
Timer
commented
Sep 27, 2018
Actually, this might be a bug: JakeChampion/fetch#657 But we still probably want the behavior of this PR by default. |
gshilin
commented
Nov 13, 2018
The same fix should be also merged to ie9.js and ie11.js |
Timer
commented
Nov 13, 2018
Feel free to send PR. |
Fixes#5124
Should we use
isomorphic-fetch? Or leave this up to the user since using the Node test environment is "advanced"?