-
-
Notifications
You must be signed in to change notification settings - Fork 26.9k
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
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. |
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 |
Hmm, |
OK |
Wait. Polyfills were an entry point in Jest. How else could we get rAF there? |
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. |
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. |
Actually, this might be a bug: JakeChampion/fetch#657 But we still probably want the behavior of this PR by default. |
The same fix should be also merged to ie9.js and ie11.js |
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"?