`promise` is undefined until first load/run

[Khartir]

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.

[ghengeveld]

Thanks! This must have been an oversight on my part. Please do open a PR ❤️

[Khartir]

Unfortunately I can only provide some tests that show the problem (see #93), I don't have a solution for it.

[ghengeveld]

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]

@Khartir I've taken the liberty to rename this issue to focus on the value of promise to always be a Promise. I've already worked out a fix which confirms that useFetch itself is not the issue. Thanks a lot for reporting this. It's a bug also because it causes TypeScript to break. One more reason to write the entire thing in TypeScript. Expect a patch release soon.

[phryneas]

Wouldn't that be problematic as a promise that never resolves/reject would cause a memory leak if it is subscribed to?

On the other side of the same problem:
The current patchnotes recommend code like this:

const { run, promise } = useAsync(...)
promise.then(onData, onError)

Wouldn't that lead to the promise being subscribed to once for each render until the promise resolves?

I'm not really sure if it was a good idea to drop the promise-return-value from run :/ It looks a lot like a foot-gun with very problematic edge cases.

[ghengeveld]

Good point. I'm not sure how the garbage collector handles this situation. At least you'll get the same unresolved promise each time, but that's still not ideal.

That snippet is indeed not optimal. Subscribing to a promise should be done in useEffect, e.g.:

const { run, promise } = useAsync(...)
useEffect(() => {
  promise.then(onData, onError)
}, [promise])

I'll update the notes accordingly. However, it won't work with <Async>{({ promise }) => promise.then(onData, onError)}</Async>, so we should come up with a better solution.

[phryneas]

Hm. Maybe we could put more focus on onResolve/onReject/onCancel and leave the promise as an internal implementation detail?
(Assuming those always just hold one callback function, haven't checked the implementation yet, just brainstorming here.)

It would be a little more clumsy to use with formik (see my example in #75 ) - but without the promise returned by run I can't think of a really straightforward way to do that anyways.

[bogdansoare]

also having this issue, run should return a promise

[ghengeveld]

@bogdansoare Could you explain your use case? That will help me think of any alternatives that might also work for you.