react-server icon indicating copy to clipboard operation
react-server copied to clipboard

Replace superagent with fetch

Open doug-wade opened this issue 9 years ago • 3 comments

Seems like fetch is rapidly becoming the standard, with pretty good polyfills for both client and node. Swapping out superagent for fetch would reduce the amount of documentation required for data loading: rather than sending in a loader argument to the page API and then explaining superagent, we'd just say "use fetch like you already do".

We might also consider a "bring your own" approach.

Migrated from #50

doug-wade avatar May 31 '16 20:05 doug-wade

I'd like to bump this feature request, because I'm curious how folks feel about it as a future direction. Would you accept a PR with it? Do you think it's a step forward or a step back?

Also, if you do think it's a good idea, I'm curious if you'd want it to be opt-in or opt-out. Right now client code opts in to the ReactServerAgent cache by requiring ReactServerAgent. If we instead intercepted all calls to fetch, it's possible that we could end up sending down some secret information in the cache that was not intended to be sent to the client. This could be alleviated by making client code opt in by doing something like import fetch from "react-server/fetch", but that would require documentation and is the kind of thing developers miss. Thoughts?

aickin avatar Jun 06 '16 21:06 aickin

Could we separate the data bundling (ReactServerAgent.cache) from the request library? Give it a clean API so that a fetch wrapper/interceptor could talk to it as easily as a superagent wrapper?

Then, I could do import SuperAgent from "react-server-superagent" or import fetch from "react-server-fetch" depending on my preference. And it would be easy to add support for other libraries.

that would require documentation and is the kind of thing developers miss.

Yeah, that's a concern. It has the advantage of being explicit, though, so the potential for surprise is lower once you know about it. But it doesn't have the same magic factor of "hey, this just works!"

gigabo avatar Jun 06 '16 21:06 gigabo

I'm not personally married to the idea of ReactServerAgent/anything-built-on-superagent. The builder pattern makes writing tests a bit of a PITA.

I think provided we maintain feature compatibility (and at least have an upgrade path) there's no reason we wouldn't entertain the idea of switching? In particular, I think it'd be nice to have plugRequest and plugResponse continue working, or at least be possible. Since fetch is such a simple API, maybe just allow users of react-server to do something like

const RS = require("react-server");
RS.setFetchImpl(myFetchImplFunction)

I know we're using ReactServerAgent internally for file uploads in at least one scenario, so it'd be good for that path to keep working as well (seems like it would, since github/polyfill seems to support it)

I also agree with the things @gigabo said, I just type slower so he beat me to them. :)

roblg avatar Jun 06 '16 21:06 roblg