Render when first value changed before mount

[mvgijssel]

I ran into a race condition using this library in combination with rxfire. The sequence of events:

1. Start mounting a component with useObservableSuspense(resource)
2. React starting suspense because resource is not yet loaded
3. The resource returns the first value from the Firestore backend
4. Component re-renders and after successfully retrieving data from the observable, it stores this data in React state packages/observable-hooks/src/use-observable-suspense.ts:19
5. The Firestore collection receives another piece of data
6. Component A finishes mounting
7. The useObservableSuspense hook will setup a subscription on the shouldUpdate$$ observable at packages/observable-hooks/src/internal/use-subscription-internal.ts:36 once the component is mounted.

The race condition happens with step 5. If the resource receives the next piece of data before mounting is completed, the shouldUpdate$$ won't have a listener setup yet and this next piece of data is never rendered in the UI.

One way of solving this problem is by using a BehaviourSubject. From the docs

One of the variants of Subjects is the BehaviorSubject, which has a notion of "the current value". It stores the latest value emitted to its consumers, and whenever a new Observer subscribes, it will immediately receive the "current value" from the BehaviorSubject.

I've also updated the postinstall hook to build the code so I can install this fork directly in my project adding the following line to my package.json:

"observable-hooks": "mvgijssel/observable-hooks#workspace=observable-hooks&commit=5917bf44cccfd77839a766415b6d7d4660b5bb8d",

[crimx]

Thanks for the PR! Introducing BehaviorSubject there will be an extra initial forceUpdate.

To prevent it I think we can give shouldUpdate$$ an extra state.

• undefined initial value. Do nothing.
• { current: value } new value.
• false triggers forceUpdate.

[crimx]

What do you think? @mvgijssel

[mvgijssel]

What do you think? @mvgijssel

Sounds good! To be clear, you want me to keep BehaiourSubject but prevent an unnecessary render by ignoring the initial undefined In the callback responsible for triggering React?

[crimx]

Yes can you update the PR?

[mvgijssel]

I've updated the PR and the associated tests, but it seems like that adding

} else if (valueRef === false) {

doesn't have an impact on the renderCount in the specs. With or without it, when using BehaviorSubject the mentions it will render one additional time. Not sure how to prevent that, so updated the specs to reflect the additional render.

[crimx]

when using BehaviorSubject the mentions it will render one additional time

I think I know why. After the Suspense re-rendering, the component is re-created, the subscription is also re-established. It's fine with the good old Subject but in the case of BehaviorSubject there will be an initial false value, so an extra re-render is triggered.

[mvgijssel]

Ah yes makes sense! Can you approve the workflow run so we can get this merged? :)

[crimx]

maybe adding a resource.shouldUpdate$$.next(undefined) before forceUpdate() should fix this? Also please change the spec back.

[crimx]

maybe adding a resource.shouldUpdate$$.next(undefined) before forceUpdate() should fix this? Also please change the spec back.

Nope. This will create race condition on multiple components consuming the same observable resource.

I'll merge this PR first and see if there are other workarounds for the optimization.

[crimx]

Fixed it by splitting valueRef$$ from shouldUpdate$$. d974012

[crimx]

@mvgijssel Could you verify that this is working as expected? Or do you know how to add a test case for this?

[crimx]

Test added 633498a .

[crimx]

observable-hooks@4.2.0 published.

[mvgijssel]

Sorry for the late reply, holidays and such 🎉. But amazing that you got it fixed! I'll install the latest version and see if it works as expected.