React: Bug: event.preventDefault is wrecking havoc with startTransition

[arackaf]

React version: 241c446

Steps To Reproduce

https://codesandbox.io/s/romantic-gates-2hrjp

1. Click "Show A" to render the lazy component.
2. Type into the input whose placeholder says it works. startTransition will call, inline loading will show for 2 seconds, then the Suspense boundary will trigger.
3. Click the "update" button - same as above - everything works
4. Now type into the input whose placeholder says it doesn't work. Note that the inline placeholder stays up for the entire time; the Suspense boundary never shows.
5. Now note that steps 2 or 3 will likely immediately show the suspense boundary. The bypassed rendering of the Suspense boundary from step 4 seems to be backed up, stuck somewhere, and is now unleashed.

Note that steps 2 and 3 are optional. No matter what, the preventDefault that's called in step 4 causes this incorrect behavior no matter whether steps 2 or 3 have been exercised.

Link to code example: see above

The current behavior

startTransition should suspend after the timeout even if event.preventDefault is called.

The expected behavior

it does not

[gaearon]

I don't understand what is happening here. This is odd.

[arackaf]

@gaearon - you don't understand what incorrect behavior my repro is showing, or are you saying you can't tell why / how React is broken? You added the "Bug" label and removed "Unconfirmed" so I assume the latter but let me know if you need me to clarify

[jddxf]

Here's what is going on:

The call of preventDefault keeps input values unchanged, so the pending discrete updates are not flushed at the end of the keydown event.
https://github.com/facebook/react/blob/241c4467eef7c2a8858c96d5dfe4e8ef84c47bad/packages/legacy-events/ReactGenericBatching.js#L41-L45

Instead, they are flushed at the beginning of the keyup event. At that time, a timeout to commit the suspended root has already been scheduled. But it is cancelled since the flush needs to prepare a fresh stack to perform work.
https://github.com/facebook/react/blob/241c4467eef7c2a8858c96d5dfe4e8ef84c47bad/packages/react-reconciler/src/ReactFiberWorkLoop.js#L1243-L1249

And no timeout would be scheduled again for some reason. Thus the placeholder is never showed.

[gaearon]

@jddxf Would you be interested in writing a failing test for this? Recent bugfixes like https://github.com/facebook/react/pull/18411 and https://github.com/facebook/react/pull/18412 might provide you with some inspiration.

[jddxf]

@gaearon I'd love to if it's not too late to do it this weekend.

[gaearon]

Anytime! In general, all issues tagged with Concurrent Mode would benefit from failing tests. Even if we don't get to fixes soon.

[arackaf]

Woo hoo - thanks everyone!!! 👊👌💯