Summary:
The change earlier in this stack makes it less safe to have ref enforcement disabled. This diff enables it by default.
ghstack-source-id: d3ab5f1b28
Pull Request resolved: https://github.com/facebook/react/pull/30716
Summary:
We previously were excessively strict about preventing functions that access refs from being returned--doing so is potentially valid for hooks, because the return value may only be used in an event or effect.
ghstack-source-id: cfa8bb1b54
Pull Request resolved: https://github.com/facebook/react/pull/30724
Summary:
Since we want to make ref-in-render errors enabled by default, we should position those errors at the location of the read. Not only will this be a better experience, but it also aligns the behavior of Forget and Flow.
This PR also cleans up the resulting error messages to not emit implementation details about place values.
ghstack-source-id: 1d11317068
Pull Request resolved: https://github.com/facebook/react/pull/30723
Test Plan:
Documents that useCallback calls interfere with it being ok for refs to escape as part of functions into jsx
ghstack-source-id: a5df427981
Pull Request resolved: https://github.com/facebook/react/pull/30714
Summary:
Refs, as stable values that the rules of react around mutability do not apply to, currently are treated as having mutable ranges, and through aliasing, this can extend the mutable range for other values and disrupt good memoization for those values. This PR excludes refs and their .current values from having mutable ranges.
Note that this is unsafe if ref access is allowed in render: if a mutable value is assigned to ref.current and then ref.current is mutated later, we won't realize that the original mutable value's range extends.
ghstack-source-id: e8f36ac25e
Pull Request resolved: https://github.com/facebook/react/pull/30713
## Summary
Flow will eventually remove the specific `React.Element` type. For most
of the code, it can be replaced with `React.MixedElement` or
`React.Node`.
When specific react elements are required, it needs to be replaced with
either `React$Element` which will trigger a `internal-type` lint error
that can be disabled project-wide, or use
`ExactReactElement_DEPRECATED`.
Fortunately in this case, this one can be replaced with just
`React.MixedElement`.
## How did you test this change?
`flow`
Prerendering in flight is similar to prerendering in Fizz. Instead of
receiving a result (the stream) immediately a promise is returned which
resolves to the stream when the prerender is complete. The promise will
reject if the flight render fatally errors otherwise it will resolve
when the render is completed or is aborted.
Supports showing the key in DevTools on the Server Component that the
key was applied to. We can also use this to reconcile to preserve
instance equality when they're reordered.
One thing that's a bit weird about this is that if you provide an
explicit key on a Server Component that alone doesn't have any
semantics. It's because we pass the key down and let the nearest child
inherit the key or get prefixed by the key.
So you might see the same key as a prefix on the child of the Server
Component too which might be a bit confusing. We could remove the prefix
from children but that might also be a bit confusing if they collide.
The div in this case doesn't have a key explicitly specified. It gets it
from the Server Component parent.
<img width="1107" alt="Screenshot 2024-08-14 at 10 06 36 PM"
src="https://github.com/user-attachments/assets/cfc517cc-e737-44c3-a1be-050049267ee2">
Overall keys get a bit confusing when you apply filter. Especially since
it's so common to actually apply the key on a Host Instance. So you
often don't see the key.
Per discussion today, adds validation against calling setState "during" passive effects. Basically, it's fine to _schedule_ setState to be called (via a timeout, listener, etc) but generally not recommended to call setState during the effect since that will trigger a cascading render.
This validation is off by default, i'm putting this up for discussion and to experiment with it internally.
ghstack-source-id: 5f385ddab5
Pull Request resolved: https://github.com/facebook/react/pull/30685
This commit updates the file locations and bulid configurations for
flight in preparation for new static entrypoints. This follows a
structure similar to Fizz which has a unified build but exports methods
from different top level entrypoints. This PR doesn't actually add the
new top level entrypoints however, that will arrive in a later update.
During local development it's common to add or remove code which may
change the cache size between renders. Add a failing test to show that
currently (without the compiled fast refresh check) this issues a
warning and reuses the cache which may have stale values.
ghstack-source-id: efdcb017ba
Pull Request resolved: https://github.com/facebook/react/pull/30662
Summary:
UseTransition is a builtin hook that returns a stable value, like useState. This PR represents that in Forget, and marks the startTransition function as stable.
ghstack-source-id: 0e76a64f2d
Pull Request resolved: https://github.com/facebook/react/pull/30681
Summary:
In theory, as I understand it, the result of a useRef will never change between renders, because we'll always provide the same ref value consistently. That means that memoization that depends on a ref value will never re-compute, so I think we could not infer it as a dependency in Forget. This diff, however, doesn't do that: it instead allows the validatePreserveExistingMemoizationGuarantees analysis to admit mismatches between explicit dependencies and implicit ones when the implicit dependency is a ref that doesn't exist in source.
ghstack-source-id: 685d859d1e
Pull Request resolved: https://github.com/facebook/react/pull/30679
This adds VirtualInstances to the tree. Each Fiber has a list of its
parent Server Components in `_debugInfo`. The algorithm is that when we
enter a set of fibers, we actually traverse level 0 of all the
`_debugInfo` in each fiber. Then level 1 of each `_debugInfo` and so on.
It would be simpler if `_debugInfo` only contained Server Component
since then we could just look at the index in the array but it actually
contains other data as well which leads to multiple passes but we don't
expect it to have a lot of levels before hitting a reified fiber.
Finally when we hit the end a traverse the fiber itself.
This lets us match consecutive `ReactComponentInfo` that are all the
same at the same level. This creates a single VirtualInstance for each
sequence. This lets the same Server Component instance that's a parent
to multiple children appear as a single Instance instead of one per
Fiber.
Since a Server Component's result can be rendered in more than one place
there's not a 1:1 mapping though. If it is in different parents or if
the sequence is interrupted, then it gets split into two different
instances with the same `ReactComponentInfo` data.
The real interesting case is what happens during updates because this
algorithm means that a Fiber can become reparented during an update to
end up in a different VirtualInstance. The ideal would maybe be that the
frontend could deal with this reparenting but instead I basically just
unmount the previous instance (and its children) and mount a new
instance which leads to some interesting scenarios. This is inline with
the strategy I was intending to pursue anyway where instances are
reconciled against the previous children of the same parent instead of
the `fiberToFiberInstance` map - which would let us get rid of that map.
In that case the model is resilient to Fiber being in more than one
place at a time.
However this unmount/remount does mean that we can lose selection when
this happens. We could maybe do something like using the tracked path
like I did for component filters. Ideally it's a weird edge case though
because you'd typically not have it. The main case that it happens now
is for reorders of list of server components. In that case basically all
the children move between server components while the server components
themselves stay in place. We should really include the key in server
components so that we can reconcile them using the key to handle
reorders which would solve the common case anyway.
I convert the name to the `Env(Name)` pattern which allows the
Environment Name to be used as a badge.
<img width="1105" alt="Screenshot 2024-08-13 at 9 55 29 PM"
src="https://github.com/user-attachments/assets/323c20ba-b655-4ee8-84fa-8233f55d2999">
(Screenshot is with #30667. I haven't tried it with the alternative
fix.)
---------
Co-authored-by: Ruslan Lesiutin <rdlesyutin@gmail.com>
Alternative to https://github.com/facebook/react/pull/30667.
Basically wrap every section in a `div` with the same class, and only
apply `border-bottom` for every instance, except for the last child. We
are paying some cost by having more divs, but thats more explicit.
When synchronously aborting in a non-async Function Component if you
throw after aborting the task would error rather than abort because
React never observed the AbortSignal.
Using a sigil to throw after aborting during render isn't effective b/c
the user code itself could throw so insteead we just read the request
status. This is ok b/c we don't expect any tasks to still be pending
after the currently running task finishes.
However I found one instance where that wasn't true related to
serializing thenables which I've fixed so we may find other cases. If we
do, though it's almost certainly a bug in our task bookkeeping so we
should just fix it if it comes up.
I also updated `abort` to not set the status to ABORTING unless the
status was OPEN. we don't want to ever leave CLOSED or CLOSING status
When I implemented the ability to abort synchronoulsy in flight I made
it possible for erroring async server components to cause an unhandled
rejection error. In the current implementation if you abort during the
synchronous phase of a Function Component and then throw an error in the
synchronous phase React will not attach any promise handlers because it
short circuits the thenable treatment and throws an AbortSigil instead.
This change updates the rendering logic to ignore the rejecting
component.
There was a comment that it's not safe to walk the unmounted fiber tree
which I'm not sure is correct or not but we need to walk the instance
tree to be able to clean up virtual instances anyway. We already walk
the instance tree to clean up "remaining instances".
This is also simpler because we don't need to special case Suspense
boundaries. We simply clean up whatever branch we had before.
The ultimate goal is to also walk the instance tree for updates so we
don't need a fiber to instance map.
## Summary
As promised on https://github.com/facebook/react/pull/29627, this
creates a unit test for the `findNodeHandle` error that prevents
developers from calling it within render methods.
## How did you test this change?
```
$ yarn test ReactFabric-test.internal.js
```
Summary:
Builds support for macros that are invoked as methods rather than just function calls or jsx.
We now record macros as a schema that represents arbitrary member expressions including wildcards (so we can support, e.g., myMacro.*.foo.bar). When examining PropertyLoads in the macro memoization stage, we build up a map of partially-satisfied macro patterns until we determine that the pattern has been fully satisfied, at which point we treat the result of the PropertyLoad as a macro value.
ghstack-source-id: d78d9ba704
Pull Request resolved: https://github.com/facebook/react/pull/30589
> [!NOTE]
> The `latest` tag is published by default if no tag is specified, which
> is what we had done since the first release of the compiler
In my last PR to auto publish compiler releases I had added the
experimental tag to be used in publishing. However because we had
already previously published to the latest tag (which is non-removable)
this means that the `latest` tag is pinned to an old version. That makes
untagged installs of the compiler default to that old version instead of
whatever is the latest.
This changes the behavior back to what it was before. Since we are still
in the experimental release of the compiler anyway it seems fine to use
the latest tag. When we reach stable, we can update this to only push to
latest for stable releases.
ghstack-source-id: 1809481b45
Pull Request resolved: https://github.com/facebook/react/pull/30666
Stacked on #30625 and #30657.
This ensures that we only create instances during the commit
reconciliation and that we don't create unnecessary instances for things
that are filtered or not mounted. This ensures that we also can rely on
the reconciliation to do all the clean up. Now everything is created and
deleted as a pair in the same pass.
Previously we were including unfiltered components in the owner stack
which probably doesn't make sense since you're intending to filter them
everywhere presumably. However, it also means that those links were
broken since you can't link into owners that don't exist in the parent
tree.
The main complication is the component filters. It relied on not
unmounting the old instances. I had to update some tests that asserted
on ids that are now shifted.
For warnings/errors tracking I now restore them back into the pending
set when they unmount. Basically it puts them back into their
"pre-commit" state. That way when they remount they’re still there.
For restoring the current selection I use the tracked path mechanism
instead of relying on the id being unchanged. This is better anyway
because if you filter out the currently selected item it's better to
select the nearest match instead of just losing the selection.
Same principle as #30555. We shouldn't be throttling the UI to make it
feel less snappy. Instead, we should use back-pressure to handle it.
Normally the browser handles it automatically with frame aligned events.
E.g. if the thread can't keep up with sync updates it doesn't send each
event but the next one. E.g. pointermove or resize.
However, it is possible that we end up queuing too many events if the
frontend can't keep up but the solution to this is the same as mentioned
in #30555. I.e. to track the last message and only send after we get a
response.
I still keep the throttle to persist the selection since that affects
the disk usage and doesn't have direct UX effects.
The main motivation for this change though is that lodash throttle
doesn't rely on timers but Date.now which makes it incompatible with
most jest helpers which means I can't write tests against these
functions properly.
This no longer uses the handleCommitFiberUnmount hook to track unmounts.
Instead, we can just unmount the DevToolsInstances that we didn't reuse.
This doesn't account for cleaning up instances that were unnecessarily
created when they weren't in the tree. I have a separate follow up for
that.
This also removes the queuing of untracking. This was added in #21523
but I don't believe it has been needed for a while because the mentioned
flushPendingErrorsAndWarningsAfterDelay hasn't called untrackFiberID for
a while so the race condition doesn't exist. It's hard to tell though
because from the issues there weren't really any repros submitted.
This is the beginning of a refactor of the DevTools Fiber backend. The
new approach is basically that we listen to each commit from Fiber and
traverse the tree - building up a filtered shadow tree. Then we send
diffs based on that tree and perform our own operations against that
instead of using Fibers as the source of truth.
Fiber diffs Elements -> Fibers. The backend diffs Fibers ->
DevToolsInstances as a side-effect it sends deltas to the front end.
This makes the algorithm resilient to a different Fiber implementation
that doesn't use pairs of Fibers (alternates) but instead stateless new
clones each time. In that world we can't treat Fibers as instances. They
can hold onto instances but they're not necessarily 1:1 themselves.
The same thing also applies to Server Components that don't have their
own instances.
The algorithm is more or less the same as React's reconciliation in
ReactChildFiber itself. However, we do a mutable update of the tree as
we go. We also cheat a bit here in the first version in that we still
have fiberToFiberInstance map and alternate which makes reorders easier.
Further down we could do the reorders by adding the previous set to a
temporary map like ChildFiber does but only if they're not already in
order.
This first bit is just about making sure that we produce correct trees.
We have fairly good test coverage already of that already.
In the next few follow ups I'll start simplifying the rest of the logic
by taking advantage of the new tree.
Sebastian Silbermann
Mike Vitousek
Sam Zhou
Josh Story
Sebastian Markbåge
Joe Savona
danandLauren Tan
Jack Pope
Timothy Yung
Mofei Zhang
hylinz
Jan Kassens