Fix for adding event listeners while in a write transaction #4446
Conversation
takameyer
force-pushed
the
andrew/rr-listeners
branch
from
f3d92f7
to
c564d55
Compare
takameyer
force-pushed
the
andrew/rr-listeners
branch
from
c564d55
to
eefbb2a
Compare
takameyer
changed the title
takameyer
changed the title
Make use of setImmediate to ensure event listeners are added after write transactions have completed. Update useQueryHook tests to give the event loop time to finish and add a listener. Fix a test in useObject render to allow event listners to fire after writes.
takameyer
force-pushed
the
andrew/rr-listeners
branch
from
eefbb2a
to
61e8e2a
Compare
| // Add this on the next tick, in case there is a write transaction occuring immediately after creation of this object | ||
| // see https://github.com/realm/realm-js/issues/4375 | ||
| setImmediate(() => { | ||
| if (object.isValid()) { |
There was a problem hiding this comment.
Are you sure this check was actually needed? If yes - would a comment be appropriate?
There was a problem hiding this comment.
It will throw if it's not valid
There was a problem hiding this comment.
I've added a comment
There was a problem hiding this comment.
Do we need to document the usage of setImmediate()? The origin of my question is the big yellow "note" in https://reactnative.dev/docs/timers.
| @@ -58,6 +58,8 @@ const useRealm = () => { | |||
|
|
|||
| const useQuery = createUseQuery(useRealm); | |||
|
|
|||
| const awaitEventLoop = () => new Promise((resolve) => setTimeout(resolve, 0)); | |||
There was a problem hiding this comment.
Could we add a comment summarising why we need to call this?
There was a problem hiding this comment.
I'm pushing an update to only use setImmediate when realm is in a write transaction. That will remove the need for this function and keep the tests understandable.
| // This replicates the issue https://github.com/realm/realm-js/issues/4375 | ||
| it("will handle multiple async transactions", async () => { | ||
| const { queryByTestId } = await setupTest({ queryType, useUseObject: true }); | ||
| const asyncEffect = async () => { |
There was a problem hiding this comment.
If this is a well known naming convention then no worries, but I thought this could be named more plainly
| const asyncEffect = async () => { | |
| const performTest = async () => { |
Only use `setImmediate` if realm is in a write transaction. Update comments to be more clear.
Co-authored-by: Tom Duncalf <tom.duncalf@mongodb.com>
takameyer
force-pushed
the
andrew/rr-listeners
branch
from
54bbd3f
to
e8f04e0
Compare
What, How & Why?
If a component is setup to render a collection that make use of both useQuery and useObject, it is possible to setup data updates in a way to set the event listeners while in a write transaction. This can occur if the writes are happing on the event loop (async) and multiple transactions occur one after the other with no break. As soon as the second write transaction is called, it will flush all event listener events. In the issue, the flushing of the event causes a component with useObject to render, which will apply an event listener on the object. This is not allowed within a write transaction.
The fix makes use of
setImmediate, which will push the event listener apply function onto the event loop. Since, in this case, the write transaction is already on the event loop, the listener will be applied immediately after the write transaction has finished, but still allow the component to finish rendering.This was also added to collections, as an object could possibly have a list property, and would end up having the same issue.
This closes # 4375
☑️ ToDos
Compatibilitylabel is updated or copied from previous entryBreakinglabel has been applied or is not necessaryIf this PR adds or changes public API's: