Summary:
To prepare for automatic effect dependencies, some codebases may want to
codemod
existing useEffect calls with no deps to include an explicit undefined
second argument
in order to preserve the "run on every render" behavior. In sufficiently
large codebases,
this may require a temporary enforcement period where all effects
provide an explicit
dependencies argument.
Outside of migration, relying on a component to render can lead to real
bugs,
especially when working with memoization.
## Summary
This PR fixes a likely incorrect condition in the
`scheduleUpdateOnFiber` function inside `ReactFiberWorkLoop.js`.
Previously, the code checked:
```js
(executionContext & RenderContext) !== NoLanes
````
However, `NoLanes` is part of the lane priority system, not the
execution context flags. The intent here seems to be to detect whether
the current execution context includes `RenderContext`, which should be
compared against `NoContext`, not `NoLanes`.
This fix replaces `NoLanes` with `NoContext` for semantic correctness
and consistency with other checks throughout the codebase.
**Fixes
[[#33169](https://github.com/facebook/react/issues/33169)](https://github.com/facebook/react/issues/33169)**
---
## How did you test this change?
I ran the following commands to validate correctness and ensure nothing
was broken:
* `yarn lint`
* `yarn linc`
* `yarn test`
* `yarn test --prod`
* `yarn flow`
* `yarn prettier`
All checks passed. Since this is a minor internal logic fix and doesn't
change public behavior or APIs, no additional tests are necessary at
this time.
## Summary
I am writing code that isn't so good, so I saw this error message many
times. It appears to have a typo. This PR fixes the typo.
## How did you test this change?
Ran the tests
Inferred effect dependencies now include optional chains.
This is a temporary solution while
https://github.com/facebook/react/pull/32099 and its followups are
worked on. Ideally, we should model reactive scope dependencies in the
IR similarly to `ComputeIR` -- dependencies should be hoisted and all
references rewritten to use the hoisted dependencies.
`
---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with [ReviewStack](https://reviewstack.dev/facebook/react/pull/33326).
* __->__ #33326
* #33325
* #32286
Inferred effect dependencies now include optional chains.
This is a temporary solution while https://github.com/facebook/react/pull/32099 and its followups are worked on. Ideally, we should model reactive scope dependencies in the IR similarly to `ComputeIR` -- dependencies should be hoisted and all references rewritten to use the hoisted dependencies.
`
When collecting scope dependencies, mark each dependency with `reactive: true | false`. This prepares for later PRs https://github.com/facebook/react/pull/33326 and https://github.com/facebook/react/pull/32099 which rewrite scope dependencies into instructions.
Note that some reactive objects may have non-reactive properties, but we do not currently track this.
Technically, state[0] is reactive and state[1] is not. Currently, both would be marked as reactive.
```js
const state = useState();
```
Stacked on #33330.
This walks the element tree to activate the various classes under
different scenarios. There are some edge case things that are a little
different since we can't express every scenario without virtual nodes.
The main thing that's still missing though is avoiding animating updates
if it can be contained to a layout or enter/exit/share if they're out of
the viewport. I.e. layout stuff.
Follow up to #33293.
This solves a race condition when boundaries are added to the batch
after the `startViewTransition` call.
This doesn't matter yet but it will once we start assigning names before
the `startViewTransition` call.
A possible alternative solution might be to ensure the names are added
synchronously in the event that adds to the batch. It's possible to keep
adding to a batch until the snapshot has happened.
I believe that these mean the same thing. We don't have to emit the
attribute if it's `none` for these cases because if there is no matching
scenario we won't apply the animation in this case.
The only case where we have to emit `none` in the attribute is for
`vt-update` because those can block updates from propagating upwards.
Follow up to #33321.
We can mark boundaries that were blocked in the prerender as postponed
but without anything to replayed inside them. That way they're not
emitted in the prerender but is unblocked when replayed.
Technically this does some unnecessary replaying of the path to the
otherwise already completed boundary but it simplifies our model by just
marking the boundary as needing replaying.
This uses the richer `serverAct` helper that we already use in other
tests.
This avoids using the `Scheduler`. We don't use that package on the
server so it doesn't make sense to simulate going through it.
Additionally, we really should be getting rid of it on the client too to
favor `postTask` polyfills.
When a new child of a fragment instance is inserted, we need to notify
the instance to keep any relevant tracking up to date. For example, we
automatically observe the new child with any active
IntersectionObserver.
For mutable renderers (DOM), we reuse the existing traversal in
`commitPlacement` that does the insertions for HostComponents. Immutable
renderers (Fabric) exit this path before the traversal though, so
currently we can't notify the fragment instances.
Here I've created a separate traversal in `commitPlacement`,
specifically for immutable renders when `enableFragmentRefs` is on.
There's an interesting case when a SuspenseList is partially prerendered
but some of the completed boundaries are blocked by rows to be resumed.
This handles it but just unblocking the future rows to avoid stalling.
However, the correct semantics will need special handling in the
postponed state.
Inferred effect dependencies now include optional chains.
This is a temporary solution while https://github.com/facebook/react/pull/32099 and its followups are worked on. Ideally, we should model reactive scope dependencies in the IR similarly to `ComputeIR` -- dependencies should be hoisted and all references rewritten to use the hoisted dependencies.
`
Inferred effect dependencies now include optional chains.
This is a temporary solution while https://github.com/facebook/react/pull/32099 and its followups are worked on. Ideally, we should model reactive scope dependencies in the IR similarly to `ComputeIR` -- dependencies should be hoisted and all references rewritten to use the hoisted dependencies.
`
Inferred effect dependencies now include optional chains.
This is a temporary solution while https://github.com/facebook/react/pull/32099 and its followups are worked on. Ideally, we should model reactive scope dependencies in the IR similarly to `ComputeIR` -- dependencies should be hoisted and all references rewritten to use the hoisted dependencies.
`
Inferred effect dependencies now include optional chains.
This is a temporary solution while https://github.com/facebook/react/pull/32099 and its followups are worked on. Ideally, we should model reactive scope dependencies in the IR similarly to `ComputeIR` -- dependencies should be hoisted and all references rewritten to use the hoisted dependencies.
`
Inferred effect dependencies now include optional chains.
This is a temporary solution while https://github.com/facebook/react/pull/32099 and its followups are worked on. Ideally, we should model reactive scope dependencies in the IR similarly to `ComputeIR` -- dependencies should be hoisted and all references rewritten to use the hoisted dependencies.
`
When collecting scope dependencies, mark each dependency with `reactive: true | false`. This prepares for later PRs https://github.com/facebook/react/pull/33326 and https://github.com/facebook/react/pull/32099 which rewrite scope dependencies into instructions.
Note that some reactive objects may have non-reactive properties, but we do not currently track this.
Technically, state[0] is reactive and state[1] is not. Currently, both would be marked as reactive.
```js
const state = useState();
```
Inferred effect dependencies now include optional chains.
This is a temporary solution while https://github.com/facebook/react/pull/32099 and its followups are worked on. Ideally, we should model reactive scope dependencies in the IR similarly to `ComputeIR` -- dependencies should be hoisted and all references rewritten to use the hoisted dependencies.
(will add more test cases)
Inferred effect dependencies now include optional chains.
This is a temporary solution while https://github.com/facebook/react/pull/32099 and its followups are worked on. Ideally, we should model reactive scope dependencies in the IR similarly to `ComputeIR` -- dependencies should be hoisted and all references rewritten to use the hoisted dependencies.
(will add more test cases)
We have many cases internally where the `containerInstance` resolves to
a comment node. `restoreRootViewTransitionName` is called when
`enableViewTransition` is on, even without introducing a
`<ViewTransition />`. So that means it can crash pages because
`containerInstance.style` is `undefined` just by turning on the flag.
This skips cancel/restore of root view transition name if a comment node is the root.
I missed setting the `keyPath` because the `renderChildrenArray` that
this is forked from doesn't need to set a path but since this is
rendered from the `SuspenseList` element it needs it.
Stacked on #33311.
When a row contains Suspense boundaries that themselves depend on CSS,
they will not resolve until the CSS has loaded on the client. We need
future rows in a list to be blocked until this happens. We could do
something in the runtime but a simpler approach is to just add those CSS
dependencies to all those boundaries as well.
To do this, we first hoist the HoistableState from a completed boundary
onto its parent row. Then when the row finishes do we hoist it onto the
next row and onto any boundaries within that row.
Stacked on #33308.
For "together" mode, we can be a self-blocking row that adds all its
boundaries to the blocked set, but there's no parent row that unblocks
it.
A particular quirk of this mode is that it's not enough to just unblock
them all on the server together. Because if one boundary downloads all
its html and then issues a complete instruction it'll appear before the
others while streaming in. What we actually want is to reveal them all
in a single batch.
This implementation takes a short cut by unblocking the rows in
`flushPartialBoundary`. That ensures that all the segments of every
boundary has a chance to flush before we start emitting any of the
complete boundary instructions. Once the last one unblocks, all the
complete boundary instructions are queued. Ideally this would be a
single `<script>` tag so that they can't be split up even if we get a
chunk containing some of them.
~A downside of this approach is that we always outline these boundaries.
We could inline them if they all complete before the parent flushes.
E.g. by checking if the row is blocked only by its own boundaries and if
all the boundaries would fit without getting outlined, then we can
inline them all at once.~ I went ahead and did this because it solves an
issue with `renderToString` where it doesn't support the script runtime
so it can only handle this if inlined.
Follow up to #33306.
If we're nested inside a SuspenseList and we have a row, then we can
point our last row to block the parent row and unblock the parent when
the last child unblocks.
We support AsyncIterable (more so when it's a cached form like in coming
from Flight) as children.
This fixes some warnings and bugs when passed to SuspenseList.
Ideally SuspenseList with `tail="hidden"` should support unblocking
before the full result has resolved but that's an optimization on top.
We also might want to change semantics for this for
`revealOrder="backwards"` so it becomes possible to stream items in
reverse order.
mofeiZ
Mofei Zhang
Jordan Brown
0xFango
Christoph Nakazawa
Sebastian Markbåge
Jack Pope
Jan Kassens
Joseph Savona