react-async: `promise` is undefined until first load/run

In Version 8 the run method no longer returns a promise. Instead the promise prop should be used. However, when using useFetch, the promise property is undefined. Here is a test that demonstrates this:

  test("defer=true allows using the promise", () => {
    let check = 0
    const component = (
      <Fetch input="/test" options={{ defer: true }}>
        {({ run, promise }) => (
          <button
            onClick={() => {
              run()
              promise
                .then(() => {
                  return (check = 1)
                })
                .catch(() => {})
            }}
          >
            run
          </button>
        )}
      </Fetch>
    )
    const { getByText } = render(component)
    fireEvent.click(getByText("run"))
    expect(check).toEqual(1)
  })

I’ll also add a Pull-Request with these failing tests.

About this issue

Original URL
State: closed
Created 5 years ago
Comments: 20 (16 by maintainers)

Most upvoted comments

@bogdansoare I feel your pain. I’m actually inclined to restore the chainability of run by returning a Thenable (PromiseLike) from it. Because it’s not an actual Promise it avoids the uncaught exception issue (although await will still throw on rejection because it treats the Thenable as a Promise). However I think we should be careful not to regress in previously resolved problems. It’s clear that the promise prop is causing more harm than good.

ghengeveld on Aug 30, 2019

also having this issue, run should return a promise

bogdansoare on Aug 30, 2019

my use case is having to perform an action after the run completes, for a concrete example I have a form inside a modal dialog and I want to close the modal when the run completes

<Form
        onSubmit={async values => {
          try {
            await run({
              name: values.name,
            });

            onClose();
          } catch (error) {
            console.log(error);
          }
        }}

bogdansoare on Aug 30, 2019

I’ve decided to just fix the issue at hand (promise being undefined), and care about the repercussions later. I’ve simply added a warning to the readme that you have to specify a rejection handler when chaining on promise.

As for the ongoing discussion, let’s take that into account while designing the future version of React Async / Async Library. One idea I have right now is to introduce a then prop which is synonymous to promise.then, but specifies a default value for onReject, so it will always be defined even if you forget to provide it yourself. Essentially this:

const noop = () => {}
const then = (onResolve, onReject = noop) => promise.then(onResolve).catch(onReject)

ghengeveld on Sep 28, 2019

I think the puzzle piece missing here is to be able to pass something to onResolve when calling run. At least, that would be great for my example in #75, where actions.setSubmitting is only available within the callback (that promise was very useful there)

function MyForm() {
  const { run } = useFetch(
    "https://example.com",
    { method: "POST" },
    { defer: true }
  );

  return (
    <div>
      <h1>My Form</h1>
      <Formik
        initialValues={{ name: "jared" }}
        onSubmit={(values, actions) => {
          run(
            { body: JSON.stringify(values) }, 
            { onResolve: () => actions.setSubmitting(false) }
          );
        }}
        render={props => (
          <form onSubmit={props.handleSubmit}>
            <Field type="text" name="name" placeholder="Name" />
            <button type="submit">Submit</button>
          </form>
        )}
      />
    </div>
  );
}

phryneas on Aug 30, 2019

Yes, thanks for that. I’ve reviewed your PR and given it some thought. I now think promise should not be undefined at the first render. Instead it should always be a Promise. We can do that by setting the initial value of promise to a Promise that never resolves or rejects. Then once run() is invoked we replace it with the actual underlying promise (which should eventually resolve or reject).

ghengeveld on Aug 28, 2019