What is the reason to patch the fetch?
This looks so unpredictable:
- works only for
getandhead(Many APIs request data viapost) - do not support XMLHttpRequest / WebSocket / WebTransport / postMessage and other side-effects
- you already have a bad practice with
console.logpatching request.json()will parse and return a new data each time, so dependent effects will recall anyway?- finally, it just super bad pattern to patch globals, it always leads to problems eventually. Next.js already holds "next" word in fetch options, which disallow to use it in feature platform updates.
So, what is the reason to patch the fetch?
I agree, patching fetch is a bad idea. This should definitely be done in another way.
This is a seriously concerning choice
This is an absurd breach of what developers expect out of something like React. Don't monkeypatch existing, standardized browser APIs. If you want to build your own fetch wrapper, fine - but invisibly altering normal browser behavior like this is asking for problems down the road.
If you don't think this is malicious (or could become so) you aren't paying attention
Wait for the RFC.
Aren't RFCs normally written before code is merged into main?
Our RFCs are almost never written before code. We iterate before we're comfortable with it. We don't just publish any first idea that comes to us.
This code is not part of a stable release. You won't see this in your app if you're just installing @latest versions.
This is better discussed once the RFC is published.
@sebmarkbage so maybe keep it in a branch, merging experiments to main is bad if you need to rollback.
We prefer working on main to avoid conflicts at the cost of needing to iterate on the design there.
Let's wait for the RFC to continue discussion there!
自以为是的react团队
React & Next might slow web development due to this stupid action. Monkeypatching it’s always bad. Especially, when you do this for native browsers API. It seems like Facebook hired some freshman’s to develop React. It can’t did a developer that has any experience or can think at least one day ahead. React and Next were my favorite technologies but now I can’t say it anymore. Damn, I really can’t believe that we make this mistake again. Didn’t we have enough problems with such monkeypatching lovers as PrototypeJS? Gosh, I just can’t believe that React team made this mega stupid action. God save the XHR…
This is bound to cause issues because it both changes the identify of globalThis.fetch as well as globalThis.fetch.toSource().
I strongly recommend against this monkey-patching and I would probably avoid using any react version that does. It's simply not the business of a DOM rendering to replace the global fetch, at least not without an opt-in mechanism.
I just want to leave this here for you to think about and for anyone coming after me to see that there is an actual caching implementation present these devs here seem to not know about: https://developer.mozilla.org/en-US/docs/Web/API/Request/cache in addition to it being a thing, it also stores data in an area that is not garbage collected by v8
you are redefining the wheel here AND pollute global objects.
the existing spec also handles this button correctly:
so just you know.. you are breaking dev tools here..
and you can't assume all api's are state-less
Again, this change is not a part of any stable release. We've heard your feedback, and will adjust the behavior changes to be opt-in (although we likely will encourage patching). We'll post an RFC with the updated proposal.
Edit: since people are doubting this comment, I'll reiterate we will make not do patching by default in React itself (but frameworks would be welcome to make or not make this choice).
+1,no intrusive changes should be made
I guess we all appreciate your response @gaearon but it doesn't answer the original question. what's the reasoning behind monkey patching the native fetch? I'm legitimately curious.
Just to circle back on this — as described earlier, we’re removing the patching ahead of the stable release. https://github.com/facebook/react/pull/28896
Note it was removed from client builds several days before this issue was filed (https://github.com/facebook/react/pull/25540) so the only remaining case was related to doing it in the RSC environment (which is where it’s being removed now).