Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suspense does not suspend on first render #242

Open
hrougier opened this issue Jan 12, 2020 · 2 comments
Open

Suspense does not suspend on first render #242

hrougier opened this issue Jan 12, 2020 · 2 comments

Comments

@hrougier
Copy link

Hi,

It looks like there is an issue with the current suspense implementation.

Considering this snippet

const { data } = useAsync({
  suspense: true,
  promise: new Promise(resolve => {
    setTimeout(() => resolve('hello'), 1000)
  }),
})

// we should be able to safely use data here as this line
// should not be reached until promise has resolved
console.log('data', data)

But we get the following output

data undefined
data hello

One of the most important purpose of suspense is to make asynchronous code feel like if it were synchronous so you don't have to deal with "data is not here yet" headaches.

By quickly looking at the implementation, this may be due to this condition

if (options.suspense && state.isPending && lastPromise.current !== neverSettle) {
  // Rely on Suspense to handle the loading state
  throw lastPromise.current
}

If lastPromise.current equals neverSettle during the first render, then it won't throw and the first render will not be suspended.

Regards

@hrougier hrougier changed the title Suspense support Suspense does not suspend on first render Jan 12, 2020
@ghengeveld
Copy link
Member

This is the part why the current Suspense support is marked experimental. It works, but it's not optimized. If you can provide a PR that fixes this I'll happily merge it.

@wolverineks
Copy link

i think tests like these describe the behavior accurately
im looking into this but can't guarantee anything

// Skip when testing for backwards-compatibility with React 16.3
  const testSuspense = Suspense ? test : test
  testSuspense("supports Suspense", async () => {
    const promiseFn = () => resolveIn(150)("done")
    const log = []
    render(
      <Suspense fallback={<div>fallback</div>}>
        <Async suspense promiseFn={promiseFn}>
          {({ data, isPending, isResolved, status }) => {
            expect(status).toBe("resolved")
            expect(data).toBe("done")
            expect(isResolved).toBe(true)
            expect(isPending).toBe(false)
            return null
          }}
        </Async>
      </Suspense>
    )
  })

  testSuspense("supports Error Boundaries", async () => {
    const promiseFn = () => rejectIn(150)(new Error("error"))
    const log = []
    render(
      <ErrorBoundary fallback={<div>fallback</div>}>
        <Async suspense promiseFn={promiseFn}>
          {({ error, isPending, isRejected, status }) => {
            expect(status).toBe("rejected")
            expect(error).toBe("error")
            expect(isRejected).toBe(true)
            expect(isPending).toBe(false)
            return null
          }}
        </Async>
      </ErrorBoundary>
    )
  })

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants