This treats workInProgressRoot work and rootWithPendingPassiveEffects
the same way. Basically as long as there's some work on the root, yield
the current task. Including passive effects. This means that passive
effects are now a continuation instead of a separate callback. This can
mean they're earlier or later than before. Later for Idle in case
there's other non-React work. Earlier for same Default if there's other
Default priority work.
This makes sense since increasing priority of the passive effects beyond
Idle doesn't really make sense for an Idle render.
However, for any given render at same priority it's more important to
complete this work than start something new.
Since we special case continuations to always yield to the browser, this
has the same effect as #31784 without implementing `requestPaint`. At
least assuming nothing else calls `requestPaint`.
<img width="587" alt="Screenshot 2024-12-14 at 5 37 37 PM"
src="https://github.com/user-attachments/assets/8641b172-8842-4191-8bf0-50cbe263a30c"
/>
DiffTrain build for [facec3ee71](https://github.com/facebook/react/commit/facec3ee71fff8b23f1e91005fce730cc96e4021)
As an alternative to #31784.
We should really just always yield each virtual task to a native task.
So that it's 1:1 with native tasks. This affects when microtasks within
each task happens. This brings us closer to native `postTask` semantics
which makes it more seamless to just use that when available.
This still doesn't yield when a task expires to protect against
starvation.
DiffTrain build for [f5077bcc92](https://github.com/facebook/react/commit/f5077bcc925aa6d0ba2ca4041c875d35e24f6266)
We introduced the `unstable_useContextWithBailout` API to run compiler
based experiments. This API was designed to be an experiment proxy for
alternative approaches which would be heavier to implement. The
experiment turned out to be inconclusive. Since most of our performance
critical usage is already optimized, we weren't able to find a clear win
with this approach.
Since we don't have further plans for this API, let's clean it up.
DiffTrain build for [909ed63e0a](https://github.com/facebook/react/commit/909ed63e0adc162a95a4704d3ed07a956dcf9cd1)
When implementing passive effects we did a pretty massive oversight.
While the passive effect is scheduled into its own scheduler task, the
scheduler doesn't always yield to the browser if it has time left. That
means that if you have a fast commit phase, it might try to squeeze in
the passive effects in the same frame but those then might end being
very heavy.
We had `requestPaint()` for this but that was only implemented for the
`isInputPending` experiment. It wasn't thought we needed it for the
regular scheduler because it yields "every frame" anyway - but it
doesn't yield every task. While the `isInputPending` experiment showed
that it wasn't actually any significant impact, and it was better to
keep shorter yield time anyway. Which is why we deleted the code.
Whatever small win it did see in some cases might have been actually due
to this issue rather than anything to do with `isInputPending` at all.
As you can see in https://github.com/facebook/react/pull/31782 we do
have this implemented in the mock scheduler and a lot of behavior that
we assert assumes that this works.
So this just implements yielding after `requestPaint` is called.
Before:
<img width="1023" alt="Screenshot 2024-12-14 at 3 40 24 PM"
src="https://github.com/user-attachments/assets/d60f4bb2-c8f8-4f91-a402-9ac25b278450"
/>
After:
<img width="1108" alt="Screenshot 2024-12-14 at 3 41 25 PM"
src="https://github.com/user-attachments/assets/170cdb90-a049-436f-9501-be3fb9bc04ca"
/>
Notice how in after the native task is split into two. It might not
always actually paint and the native scheduler might make the same
mistake and think it has enough time left but it's at least less likely
to.
We do have another way to do this. When we yield a continuation we also
yield to the native browser. This is to enable the Suspense Optimization
(currently disabled) to work. We could do the same for passive effects
and, in fact, I have a branch that does but because that requires a lot
more tests to be fixed it's a lot more invasive of a change. The nice
thing about this approach is that this is not even running in tests at
all and the tests we do have assert that this is the behavior already. 😬
DiffTrain build for [c80b336d23](https://github.com/facebook/react/commit/c80b336d23aa472b5e5910268138ac0447d6ae19)
Related to #31752.
When hydrating, we have two different ways of handling a Suspense
boundary that the server has already given up on and decided to client
render. If we have already hydrated the parent and then later this
happens, then we'll use the retry lane like any ping. If we discover
that it was already in client-render mode when we discover the Suspense
boundary for the first time, then schedule a default lane to let us
first finish the current render and then upgrade the priority to sync to
try to client render this boundary as soon as possible since we're
holding back content.
We used to use the `DefaultHydrationLane` for this but this is not
really a Hydration. It's actually a client render. If we get any other
updates flowing in from above at the same time we might as well do them
in the same pass instead of two passes. So this should be considered
more like any update.
This also means that visually the client render pass now gets painted as
a render instead of a hydration.
This show the flow of a shell being hydrated at the default priority,
then a Suspense boundary being discovered and hydrated at Idle and then
an inner boundary being discovered as client rendered which gets
upgraded to default.
<img width="1363" alt="Screenshot 2024-12-14 at 12 13 57 AM"
src="https://github.com/user-attachments/assets/a141133e-4856-4f38-a11f-f26bd00b6245"
/>
DiffTrain build for [d1dd7feabc](https://github.com/facebook/react/commit/d1dd7feabc63bf8ca61e9b3f4d78245a29ebbe9a)
When scheduling the initial root and when using
`unstable_scheduleHydration` we should use the Hydration Lanes rather
than the raw update lane. This ensures that we're always hydrating using
a Hydration Lane or the Offscreen Lane rather than other lanes getting
some random hydration in it.
This fixes an issue where updating a root while it is still hydrating
causes it to trigger client rendering when it could just hydrate and
then apply the update on top of that.
It also fixes a potential performance issue where
`unstable_scheduleHydration` gets batched with an update that then ends
up forcing an update of a boundary that requires it to rewind to do the
hydration lane anyway. Might as well just start with the hydration
without the update applied first.
I added a kill switch (`enableHydrationLaneScheduling`) just in case but
seems very safe given that using `unstable_scheduleHydration` at all is
very rare and updating the root before the shell hydrates is extremely
rare (and used to trigger a recoverable error).
DiffTrain build for [d5e8f79cf4](https://github.com/facebook/react/commit/d5e8f79cf4d11fa7eee263b3f937deecbe65ffd7)
# Summary
I'm working to get the main `react-native` package parsable by modern
Flow tooling (both `flow-bundler`, `flow-api-translator`).
This diff trivially removes some redundant Flow comment syntax in
`ReactNativeTypes.js`, which fixes parsing under these newer tools.
## How did you test this change?
Files were pasted into `react-native-github` under fbsource, where Flow
validates ✅.
DiffTrain build for [92b62f500c](https://github.com/facebook/react/commit/92b62f500c3fca44a9dc9ead936ef3bf19481f02)
The need for this was removed in
https://github.com/facebook/react/pull/30831
Since the new DevTools version has been released for a while and we
expect people to more or less auto-update. Future versions of React
don't need this.
Once we remove the remaining uses of `getInstanceFromNode` e.g. in the
deprecated internal `findDOMNode`/`findNodeHandle` and the event system,
we can completely remove the tagging of DOM nodes.
DiffTrain build for [3b597c0576](https://github.com/facebook/react/commit/3b597c0576977773910c77e075cc6d6308decb04)
To avoid GC pressure and accidentally hanging onto old trees Suspense
boundary retries are now implemented in the commit phase. I used the
Callback flag which was previously only used to schedule callbacks for
Class components. This isn't quite semantically equivalent but it's
unused and seemingly compatible.
DiffTrain build for [de68d2f4a2](https://github.com/facebook/react/commit/de68d2f4a2403ad1ef46a3036ddc1f9080640588)
When streaming SSR while hydrating React will wait for Suspense
boundaries to be revealed by the SSR stream before attempting to hydrate
them. The rationale here is that the Server render is likely further
ahead of whatever the client would produce so waiting to let the server
stream in the UI is preferable to retrying on the client and possibly
delaying how quickly the primary content becomes available. However If
the connection closes early (user hits stop for instance) or there is a
server error which prevents additional HTML from being delivered to the
client this can put React into a broken state where the boundary never
resolves nor errors and the hydration never retries that boundary
freezing it in it's fallback state.
Once the document has fully loaded we know there is not way any
additional Suspense boundaries can arrive. This update changes react-dom
on the client to schedule client renders for any unfinished Suspense
boundaries upon document loading.
The technique for client rendering a fallback is pretty straight
forward. When hydrating a Suspense boundary if the Document is in
'complete' readyState we interpret pending boundaries as fallback
boundaries. If the readyState is not 'complete' we register an event to
retry the boundary when the DOMContentLoaded event fires.
To test this I needed JSDOM to model readyState. We previously had a
temporary implementation of readyState for SSR streaming but I ended up
implementing this as a mock of JSDOM that implements a fake readyState
that is mutable. It starts off in 'loading' readyState and you can
advance it by mutating document.readyState. You can also reset it to
'loading'. It fires events when changing states.
This seems like the least invasive way to get closer-to-real-browser
behavior in a way that won't require remembering this subtle detail
every time you create a test that asserts Suspense resolution order.
DiffTrain build for [16d2bbbd1f](https://github.com/facebook/react/commit/16d2bbbd1f1617d636ea0fd271b902a12a763c27)
A long standing issue for React has been that if you reorder stateful
nodes, they may lose their state and reload. The thing moving loses its
state. There's no way to solve this in general where two stateful nodes
swap.
The [`moveBefore()`
proposal](https://chromestatus.com/feature/5135990159835136?gate=5177450351558656)
has now moved to
[intent-to-ship](https://groups.google.com/a/chromium.org/g/blink-dev/c/YE_xLH6MkRs/m/_7CD0NYMAAAJ).
This function is kind of like `insertBefore` but preserves state.
There's [a demo here](https://state-preserving-atomic-move.glitch.me/).
Ideally we'd port this demo to a fixture so we can try it.
Currently this flag is always off - even in experimental. That's because
this is still behind a Chrome flag so it's a little early to turn it on
even in experimental. So you need a custom build. It's on in RN but only
because it doesn't apply there which makes it easier to tell that it's
safe to ship once it's on everywhere else.
The other reason it's still off is because there's currently a semantic
breaking change. `moveBefore()` errors if both nodes are disconnected.
That happens if we're inside a completely disconnected React root.
That's not usually how you should use React because it means effects
can't read layout etc. However, it is currently supported. To handle
this we'd have to try/catch the `moveBefore` to handle this case but we
hope this semantic will change before it ships. Before we turn this on
in experimental we either have to wait for the implementation to not
error in the disconnected-disconnected case in Chrome or we'd have to
add try/catch.
DiffTrain build for [aba370f1e4](https://github.com/facebook/react/commit/aba370f1e45d21f19f33c04c33fc99fb3d0109e5)
Fixes a bug with the experimental `useResourceEffect` hook where we
would compare the wrong deps when there happened to be another kind of
effect preceding the ResourceEffect. To do this correctly we need to add
a pointer to the ResourceEffect's identity on the update.
I also unified the previously separate push effect impls for resource
effects since they are always pushed together as a unit.
DiffTrain build for [c11c9510fa](https://github.com/facebook/react/commit/c11c9510fa14bbd87053685c19bfdfec2f427f49)
This lets us track separately if something was suspended on an Action
using useActionState rather than suspended on Data.
This approach feels quite bloated and it seems like we'd eventually
might want to read more information about the Promise that suspended and
the context it suspended in. As a more general reason for suspending.
The way useActionState works in combination with the prewarming is quite
unfortunate because 1) it renders blocking to update the isPending flag
whether you use it or not 2) it prewarms and suspends the useActionState
3) then it does another third render to get back into the useActionState
position again.
DiffTrain build for [92c0f5f85f](https://github.com/facebook/react/commit/92c0f5f85fed42024b17bf6595291f9f5d6e8734)
## Summary
This fixes a typo in the error that gets reported when Float errors
while hoisting a style tag that does not contain both `precedence` and
`href`. There was a typo in _conflict_ and the last part of the sentence
doesn't make sense. I assume it wasn't needed since the message already
suggests moving the style tag to the head manually.
DiffTrain build for [63cde684f5](https://github.com/facebook/react/commit/63cde684f5340b1ca73f6244501aac1c3d2c92a8)
In preparation for the next RC, I set this feature flag to true
everywhere. I did not delete the feature flag yet, in case there are yet
more bugs to be discovered.
I also didn't remove the dynamic feature flag from the Meta builds; I'll
let the Meta folks handle that.
DiffTrain build for [988e217670](https://github.com/facebook/react/commit/988e2176702fca9b25113d9a8a3e7e3f484e16f2)
We've previously failed to land this change due to some internal apps
seeing infinite render loops due to external store state updates during
render. It turns out that since the `renderWasConcurrent` var was moved
into the do block, the sync render triggered from the external store
check was stuck with a `RootSuspended` `exitStatus`. So this is not
unique to sibling prerendering but more generally related to how we
handle update to a sync external store during render.
We've tested this build against local repros which now render without
crashes. We will try to add a unit test to cover the scenario as well.
---------
Co-authored-by: Andrew Clark <git@andrewclark.io>
Co-authored-by: Rick Hanlon <rickhanlonii@fb.com>
DiffTrain build for [989af12f72](https://github.com/facebook/react/commit/989af12f72080c17db03ead91d99b6394a215564)
## Summary
We're seeing certain situations in React Native development where ref
callbacks in `<Activity mode="hidden">` are sometimes invoked exactly
once with `null` without ever being called with a "current" value.
This violates the contract for refs because refs are expected to always
attach before detach (and to always eventually detach after attach).
This is *particularly* bad for refs that return cleanup functions,
because refs that return cleanup functions expect to never be invoked
with `null`. This bug causes such refs to be invoked with `null`
(because since `safelyAttachRef` was never called, `safelyDetachRef`
thinks the ref does not return a cleanup function and invokes it with
`null`).
This fix makes use of `offscreenSubtreeWasHidden` in
`commitDeletionEffectsOnFiber`, similar to how
https://github.com/facebook/react/commit/ec52a5698e2dfea7050a0b015f0b79abfb2d81b7
did this for `commitDeletionEffectsOnFiber`.
## How did you test this change?
We were able to isolate the repro steps to isolate the React Native
experimental changes. However, the repro steps depend on Fast Refresh.
```
function callbackRef(current) {
// Called once with `current` as null, upon triggering Fast Refresh.
}
<Activity mode="hidden">
<View ref={callbackRef} />;
</Activity>
```
Ideally, we would have a unit test that verifies this behavior without
Fast Refresh. (We have evidence that this bug occurs without Fast
Refresh in real product implementations. However, we have not
successfully deduced the root cause, yet.)
This PR currently includes a unit test that reproduces the Fast Refresh
scenario, which is also demonstrated in this CodeSandbox:
https://codesandbox.io/p/sandbox/hungry-darkness-33wxy7
Verified unit tests pass:
```
$ yarn
$ yarn test
# Run with `-r=www-classic` for `enableScopeAPI` tests.
$ yarn test -r=www-classic
```
Verified on the internal React Native development branch that the bug no
longer repros.
---------
Co-authored-by: Rick Hanlon <rickhanlonii@fb.com>
DiffTrain build for [ea3ac58669](https://github.com/facebook/react/commit/ea3ac586693014e882655728fc8396ecb1d6cf6e)
This reverts commit 6c4bbc7832.
It looked like the bug we found on the original land was related to
broken product code. But through landing #31268 we found additional bugs
internally. Since disabling the feature flag does not fix the bugs, we
have to revert again to unblock the sync. We can continue to debug with
our internal build.
DiffTrain build for [cae764ce81](https://github.com/facebook/react/commit/cae764ce81b1bd6c418e9e23651794b6b09208e8)
<!--
Thanks for submitting a pull request!
We appreciate you spending the time to work on these changes. Please
provide enough information so that others can review your pull request.
The three fields below are mandatory.
Before submitting a pull request, please make sure the following is
done:
1. Fork [the repository](https://github.com/facebook/react) and create
your branch from `main`.
2. Run `yarn` in the repository root.
3. If you've fixed a bug or added code that should be tested, add tests!
4. Ensure the test suite passes (`yarn test`). Tip: `yarn test --watch
TestName` is helpful in development.
5. Run `yarn test --prod` to test in the production environment. It
supports the same options as `yarn test`.
6. If you need a debugger, run `yarn test --debug --watch TestName`,
open `chrome://inspect`, and press "Inspect".
7. Format your code with
[prettier](https://github.com/prettier/prettier) (`yarn prettier`).
8. Make sure your code lints (`yarn lint`). Tip: `yarn linc` to only
check changed files.
9. Run the [Flow](https://flowtype.org/) type checks (`yarn flow`).
10. If you haven't already, complete the CLA.
Learn more about contributing:
https://reactjs.org/docs/how-to-contribute.html
-->
## Summary
In order to adopt react 19's ref-as-prop model, Flow needs to eliminate
all the places where they are treated differently.
`React.AbstractComponent` is the worst example of this, and we need to
eliminate it.
This PR eliminates them from the react repo, and only keeps the one that
has 1 argument of props.
## How did you test this change?
yarn flow
DiffTrain build for [45804af18d](https://github.com/facebook/react/commit/45804af18d589fd2c181f3b020f07661c46b73ea)
## Summary
The React Native Renderer exports a
`__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED` property with a
single method that has no remaining call sites:
`computeComponentStackForErrorReporting`
This PR cleans up this unused export.
## How did you test this change?
```
$ yarn
$ yarn flow fabric
$ yarn test
```
DiffTrain build for [a3d9ea05bf](https://github.com/facebook/react/commit/a3d9ea05bf01f3c3d7aedc2d938c581ad11fd14a)
rickhanlonii
eps1lon
sebmarkbage
jackpope
noahlemen
hoxyq
gnoff
poteto
acdlite
sammy-SC
sophiebits
kassens
yungsters
SamChou19815