Skip to content

Disable observable timeouts for suspense - #525

Merged
jhuleatt merged 2 commits into
FirebaseExtended:mainfrom
Mythie:remove-suspense-timeout
Jun 27, 2023
Merged

jhuleatt merged 2 commits into
FirebaseExtended:mainfrom
Mythie:remove-suspense-timeout

Conversation

@Mythie

@Mythie Mythie commented Jun 10, 2022

Copy link
Copy Markdown
Contributor

Description

When using reactfire with react 17, any request that exceeds the default timeout of 30 seconds will then trigger the timeout handler that resets the observable.

Unfortunately, this resetting of the observable also means that the firstEmission promise never gets resolved leading to an infinite fallback state with suspense.

To resolve this, we pass along whether or not we are in suspense mode when creating an observable so we may conditionally choose to allow the timeout for resetting the observable.

@google-cla

google-cla Bot commented Jun 10, 2022

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@Mythie
Mythie marked this pull request as ready for review June 15, 2022 04:01
@jhuleatt
jhuleatt requested a review from jamesdaniels August 4, 2022 14:13

@jhuleatt jhuleatt left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a really interesting fix, thank you @Mythie! Given the number of thumbs-ups on this PR and that the tests pass, I'm going to merge it

@jhuleatt
jhuleatt merged commit 4329c43 into FirebaseExtended:main Jun 27, 2023
@FirebaseExtended FirebaseExtended locked and limited conversation to collaborators Jul 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants