refetch should return proper value in lazy get

[smilexu]

Is your feature request related to a problem? Please describe. I`d like to use react-select async select component with restful-react. For async loading options, we need to return proper options via loadOptions callback, the code is like below.

<AsyncSelect
  ...
  loadOptions={(input, callback) => {
    const result  = doQueryByInput(input);
    callback(buildOptionsFromResult(result));
  }}
/>

The problem is that refetch will not return any value, I have no idea how to return proper options by callback.

Describe the solution you'd like I'd like to get value from refetch, code like below

  const { data, refetch } = useGet({
    path: '/search-something',
    lazy: true,
  });

...

<AsyncSelect
  ...
  loadOptions={(input, callback) => {
    refetch({
      queryParams: buildQueryParams(input),
    }).then((response) => {
      callback(buildOptions(response));
    });
  }}
/>

Describe alternatives you've considered No other alternatives so far, but if you have any idea can archive that, it would be also appreciate.

Additional context No

[fabien0102]

Hello, we are also using react-select internally, so I just checked 😋

We are using a simple fetch with a custom generator instead of a react hook.

Something like this: https://github.com/contiamo/restful-react/blob/61604ee50626b3c910224225cf4dd92cf4b260b0/examples/restful-react.config.js#L20-L40

But this is mainly to have types from the open-api specs, otherwise a simple fetcher is more appropriate than restful-react for this usecase ;) (you just need to export your base to compose your url)

[smilexu]

Not sure if I understood your comment correctly, I already implement my project based on restful-react which handles error, auth etc, do not want to add a more solution (use other fetcher)only for async load scenario. So I will prefer to have some restful-react build-in solution to solve that if possible. apollo-graphql is a good example, we can reset params when refetching, but sure, it is different implementation.

[fabien0102]

Make sense, for now refetch already take some arguments, but is totally bind to the state (because it's a refetch, not a fetch).

This would be easy to return some value from refetch (I just checked the codebase), but will this make the API a bit funky, since refetch was meant to refetching, not fetching… (so you will have some useless rerender for the loading for example)

I sadly don't have a lot of time right now to play with this, but you can try to play around locally and submit a PR. I think a cleaner solution will be to introduce a useFetch, to avoid the conflict between the two API (data from state vs data from promise).

Maybe something like this

const getCollection = useFetch("GET", "/mycollection", {/* options, etc… */})

getCollection({/* params */}) // Promise<CollectionRes>

So you get all the context (basePath mainly) from restful-react and can also propagate the error the global handler if localErrorOnly = true

My main remaining question is about the Promise type and how to deal with the Error type, sadly it's not really possible to type the catch, and I think having the error type is something nice (even if it's usually forgotten ^^)

[amacleay-cohere]

I'll add in one more detail here which I think is pretty pertinent and has tripped up a number of developers (including me!) several times.

Suppose you have a search endpoint which should get invoked when a user clicks a button, and that dispatches some action. If the search endpoint is a POST and takes a body, it looks like this:

const { data, mutate } = useMutate("POST", "/search");
const [input, setInput] = useState("");

return (
  <div>
    <h1>{data?.results?.length || 0} results</h1>
    <input value={input} onChange={e => { setInput(e.target.currentValue) }}>
    <button onClick={async () => {
      const newResult = await mutate({ input });
      if (newResult.results.length > 1) {
        alert("More results");  // Maybe some workflow driving code here
      }
    }}>
      Search
    </button>
  </div>
);

The pattern in which the output is used both in the render in a component, as well as in some callback code, is not the primary pattern but isn't uncommon.

Now suppose that we're changing this to look at a search endpoint which happens to be a GET and take query parameters. Ideally I'd hardly have to change it at all:

const { data, refetch } = useGet("/search");
const [input, setInput] = useState("");

return (
  <div>
    <h1>{data?.results?.length || 0} results</h1>
    <input value={input} onChange={e => { setInput(e.target.currentValue) }}>
    <button onClick={async () => {
      const newResult = await refetch({ queryParams: { input } });
      if (newResult.results.length > 1) {
        alert("More results");  // Maybe some workflow driving code here
      }
    }}>
      Search
    </button>
  </div>
);

but without refetch returning the result, we need to split the call and the result processing in a kind of unnatural way:

const { data, refetch } = useGet("/search");
const [input, setInput] = useState("");

useEffect(() => {
  if (data.results.length > 1) {
    alert("More results");  // Maybe some workflow driving code here
  }
}, [data]);

return (
  <div>
    <h1>{data?.results?.length || 0} results</h1>
    <input value={input} onChange={e => { setInput(e.target.currentValue) }}>
    <button onClick={() => {
      refetch({ queryParams: { input } });
    }}>
      Search
    </button>
  </div>
);

My main argument that this would be useful boils down to these two points:

• It isn't that uncommon to want to use the results of a refetch both in the rendered component and inline at the time of the call
• The signature of refetch seems capriciously different from the mutate signature in refactors like this

[fabien0102]

Hi @amacleay-cohere, The "intended design" for this kind of usecase was to not use refetch, refetch was suppose to just refetch ^^ But I totally get the confusion.

If we take your example, with the behaviour of sending the value on click, this will be the implementation:

import react, { useState } from "react";
import { useGet } from "restful-react";

export const myComponent = () => {
  const [draftInput, setDraftInput] = useState("");
  const [input, setInput] = useState("");
  const { data } = useGet("/search", { queryParams: { input } });

  return (
    <div>
      <h1>{data?.results?.length || 0} results</h1>
      <input
        value={input}
        onChange={(e) => {
          setDraftInput(e.target.currentValue);
        }}
      />
      <button
        onClick={() => {
          setInput(draftInput);
        }}
      >
        Search
      </button>
    </div>
  );
};

Since useGet will automatically refetch when a props change, you can just give input as parameter and everything should follow.

[amacleay-cohere]

Fair enough! I totally appreciate the core use case, and the intended behavior certainly works for probably the majority of use cases.

My observation is that, in the wild, we regularly run into these use cases at the margins with one or both of these two characteristics:

• Data that both renders and has a meaningful side effect
  • In my toy example above, that's the header and the alert. In our actual use cases, this is more often triggering a modal, or kicking off a workflow step
• Non-trivial interaction among server-generated data
  • Not represented in my example at all, but often a big factor in real use cases. It's simple enough to move one calculation into a useEffect, but if you happen to have multiple serial server calls, that becomes much harder to track, and if the origination of the server call is user-generated, it's particularly thorny. Ideally in those cases, you'd push the complexity into the server, but you may not always have control over the server API

The real use case that I'm concerned about looks more like this: we have two different server calls, one that does an initial search and then one which gets additional data from the search results. Both are returning data that gets displayed in the component, but depending on the result of the second one we may or may not trigger a modal:

const { data: searchResults, loading: searchLoading, mutate: doSearch } = useMutate("POST", "/search");
// lazy because there's nothing to search until there are results
const { data: resultDetail, loading: detailsLoading, refetch: fetchDetails } = useGet("/details", { lazy: true })

const [input, setInput] = useState("");
const [alertMessage, setAlertMessage] = useState("");

useEffect(() => {
  if (searchResults?.results) {
    fetchDetails({ queryParams: { ids: searchResults.results.map(x => x.id).join(",") } });
  }
}, [searchResults, fetchDetails]);
useEffect(() => {
  if (resultDetail.results) {
    const maybeMessage = resultDetail.results.find(x => x.alertMessage)?.alertMessage;
    setAlertMessage(maybeMessage || "");
  }
}, [resultDetail]);

return (
  <div>
    <h1>{data?.results?.length || 0} results</h1>
    <input value={input} onChange={e => { setInput(e.target.currentValue) }}>
    <button disabled={searchLoading || detailsLoading} onClick={() => {
      doSearch({ input });
    }}>
      Search
    </button>
    <AlertModal message={alertMessage} open={Boolean(alertMessage)} />
    {/* imagine much more interesting stuff here, like a table of results */}
  </div>
);

An issue with this implementation: searchLoading || detailsLoading for the disabled state will actually allow the button to flicker back into an active state briefly, but trying to externalize a state variable for "user hit the button but we are still waiting for details to load" is somewhat difficult.

If the /details endpoint were a post, we wouldn't need all this indirection through the separate useEffects:

const { data: searchResults, mutate: doSearch } = useMutate("POST", "/search");
const { data: resultDetail, mutate: fetchDetails } = useMutate("POST", "/details")

const [input, setInput] = useState("");
const [alertMessage, setAlertMessage] = useState("");

const [searchButtonLoading, setSearchButtonLoading] = useState(false);

return (
  <div>
    <h1>{data?.results?.length || 0} results</h1>
    <input value={input} onChange={e => { setInput(e.target.currentValue) }}>
    <button disabled={searchButtonLoading} onClick={async () => {
      setSearchButtonLoading(true);
      const resp = await doSearch({ input });
      if (resp && resp.results) {
        const detailResp = await fetchDetails({ ids: resp.results.map(x => x.id) });
        if (detailResp.results) {
          const maybeMessage = detailResp.results.find(x => x.alertMessage)?.alertMessage;
          setAlertMessage(maybeMessage || "");
      }
      setSearchButtonLoading(false);
    }}>
      Search
    </button>
    <AlertModal message={alertMessage} open={Boolean(alertMessage)} />
    {/* imagine much more interesting stuff here, like a table of results */}
  </div>
);

This is a little more illustrative: we have multiple calls dispatched by a user action. The result of both calls individually shows up on the render (elided in the example code), but we also want to provide feedback directly from that action.

Sorry for the novel - I'm having a hard time figuring out a clear but also concise way to express what I mean! I did open a PR with the relevant changes in #376 though I recognize that this question about the interface for refetch is important to get right.

Side note: I really appreciate all the great work you maintainers have put into making this project so great!

[fabien0102]

So, let’s try to see if we can use restful-react for this kind of usecases (after, to be totally honnest with you, when this is becoming a bit too tricky, I do prefer put this in good old redux + rxjs with restful-react just there to generate the correct types 😁)

Let’s use a computed lazy and bring back queryParams to the query (so no extra request and no need for the useEffect)

diff --git a/playground.js b/playground.js
index efa0885..1bfc11e 100644
--- a/playground.js
+++ b/playground.js
@@ -1,15 +1,13 @@
 const { data: searchResults, loading: searchLoading, mutate: doSearch } = useMutate("POST", "/search");
 // lazy because there's nothing to search until there are results
-const { data: resultDetail, loading: detailsLoading, refetch: fetchDetails } = useGet("/details", { lazy: true })
+const { data: resultDetail, loading: detailsLoading, refetch: fetchDetails } = useGet("/details", {
+  lazy: Boolean(searchResults.results),
+  queryParams: { ids: searchResults.results.map(x => x.id).join(",") }
+})
 
 const [input, setInput] = useState("");
 const [alertMessage, setAlertMessage] = useState("");
 
-useEffect(() => {
-  if (searchResults?.results) {
-    fetchDetails({ queryParams: { ids: searchResults.results.map(x => x.id).join(",") } });
-  }
-}, [searchResults, fetchDetails]);
 useEffect(() => {
   if (resultDetail.results) {
     const maybeMessage = resultDetail.results.find(x => x.alertMessage)?.alertMessage;

(anyway, you PR looks good 😅 I just like challenging ^^)