Wait for pending transactions from ReactServerRenderingTransaction #7033
Conversation
ghost
added
|
@spicyj can you please review this? |
| }, | ||
| }); | ||
| expect( | ||
| ReactServerRendering.renderToString.bind( |
There was a problem hiding this comment.
Can we just use an arrow here? I think this would be a bit less confusing.
There was a problem hiding this comment.
Do you want me to change the other instances of this? I was just following the format of the other tests in this file. (see ReactServerRendering-test.js#L265-L268)
There was a problem hiding this comment.
Ah okay, let's keep it then.
|
Let’s make those small changes and get this in. The fix is a bit ugly but the whole “revert injection” thing is problematic by itself, and at least we have a test now for this. Thanks! |
|
Yeah, it's definitely not a long term solution. Ideally pending transactions would never be processed by a batching strategy other than the one it was batched with, but that seems like it would involve more work towards how transactions are processed. I'll get this fixed tomorrow morning 👍 |
aweary
force-pushed
the
nested-ssr-transactions
branch
from
a0eb88d
to
520f5dc
Compare
|
@gaearon I made those changes you requested |
|
Thank you for numerous contributions! |
Thanks for the feedback! Looking forward to making more contributions when I can 😄 |
|
@aweary Hi there – this fix is listed as part of the 15.2.1 changelog, but unless I'm reading the source wrong, it does not seem to have been actually pulled in. Just wanted to give a heads up so that it can be properly included in the 15.2.2 release. (Btw, you saved my skin with this fix). cc @zpao |
|
@jcfrancisco seems like you're right, CC @gaearon since he merged this in initially. |
|
Oops! I think I know what happened. I'll make sure this goes into the next release. |
|
Thanks @zpao! |
(cherry picked from commit b6e1eb2)
Resolves #6423
The issue I found with #6423 was that there were pending actions from a
ReactServerRenderingTransaction, so after it reverted the batching strategy after the first transaction any updates that the pending transaction(s) may have had queued were batched withReactDefaultBatchingStrategywhich caused it to fail.This PR simply tracks the number of pending
ReactServerRenderingTransactioninstances and only reverts the batching strategy once the final transaction has finished.