ideas icon indicating copy to clipboard operation
ideas copied to clipboard

useLoadable

Open hanford opened this issue 5 years ago • 8 comments

I love how GraphQL <QueryRender /> have render props that return

({ loading, props, error, retry })

I often find myself writing the same logic around form submit buttons and other various UI where I need to know whether my async function is loading, had an error or the result.

I'm proposing an API like this:

const sleep = time => () => new Promise(done => setTimeout(done, time));

function App() {
  const [onClick, { loading, error }] = useAsyncFunction(sleep(500));

  return (
    <>
      <pre>
        error: {JSON.stringify(error)}
        <br />
        loading: {JSON.stringify(loading)}
      </pre>
      <button onClick={onClick}>{loading ? "Loading..." : "Load"}</button>
    </>
  );
}

I think we should also support a delayMs option that will tell our async function only resolve after a given ms .. it should work like this:

const sleep = () => Promise.resolve();

function App() {
  const [onClick, { loading, error }] = useAsyncFunction(sleep, { delayMs: 500 });

  return (
    <>
      <pre>
        error: {JSON.stringify(error)}
        <br />
        loading: {JSON.stringify(loading)}
      </pre>
      <button onClick={onClick}>{loading ? "Loading..." : "Load"}</button>
    </>
  );
}

I have most of the code working, I'll just need to add type defintions and some better testing in this repo: https://github.com/hanford/async-function

Once that's done, I'll transfer the ownership (aiming to be complete tomorrow morning)

hanford avatar Oct 29 '18 04:10 hanford

See also #24

j-f1 avatar Oct 29 '18 13:10 j-f1

This is done, I can transfer ownership but want to hear what the thoughts are in #24 first.

hanford avatar Oct 29 '18 18:10 hanford

cc @jamiebuilds @fouad @jamesplease thoughts?

hanford avatar Oct 30 '18 16:10 hanford

I like this but useAsync might be a bit too generic. I'd be concerned about a breaking change if Suspense has a more recommended way of handling async/await-like suspension in hooks. It reminds me of the react-loadable so I'd suggest two changes:

1. Rename to useLoadable

There are a few other react-loadable types that would be nice, e.g. loaded and timedOut in output & timeout in options

2. Output order should be [{ loading, loaded, error }, onLoad] like useState

I can understand [trigger, value] as the rationale for the current ordering but I think it might be more confusing than helpful, e.g. node libraries that switched around order for cb and err

open to pushback/other ideas!

fouad avatar Oct 31 '18 04:10 fouad

I'll rename to loadable, I also think switching the order is a good idea 🙌

I'm a little on the fence about loaded (seems easy to infer from loading, also afraid of assuming people will only want to run once) and timeout because it can easily be added if desired.

It makes me think that adding some separate hooks might be better than increasing the scope of this one.

anyways - appreciate the feedback and will update this thread once I've updated the code on my end!

hanford avatar Oct 31 '18 15:10 hanford

np! maybe @jamiebuilds will have a useful opinion on loaded vs other options (e.g. data or value)

fouad avatar Oct 31 '18 16:10 fouad

@fouad sort of torn on adding the timeout functionality to useLoadable .. If useLoadable included the option to timeout, I'd expect it to subsequently cancel whatever async function is happening and that seems like a can of worms.

I'd rather defer the module consumer add some sort of composition on their end

const [{loading, error}, trigger] = useLoader(useTimeout(fetch(..), 3000))

You can check out the code here if you're curious how it would work: https://codesandbox.io/s/k996n20p55

I've also updated the repo with the other changes from above: https://github.com/hanford/loadable

hanford avatar Oct 31 '18 16:10 hanford

You also need to provide boolean props like isError and isLoaded, because you can have Promises that resolve (and/or reject) with null (or undefined). So you aren't able to tell if they are "done" by looking only at data and error. And if you only check for loading === false, you cannot tell if the promise already started to run or if it has finished. And if it has finished, you cannot tell if it was successful or erroneous.

For example we have several HTTP delete requests, that don't return anything on success. Of course I could refactor our API layer to always return true, but I'd say that this is something for the hook. Because when using a normal promise you can simply call .then(() => ...) without any arguments, and you can be sure that it was successful.

benneq avatar Jan 21 '19 10:01 benneq