Commit Graph
19393 Commits
Author SHA1 Message Date
Mike Vitousek cd1cb37dbd Update base for Update on "[compiler] Improve handling of refs"
Summary:
This change expands our handling of refs to build an understanding of nested refs within objects and functions that may return refs. It builds a special-purpose type system within the ref analysis that gives a very lightweight structural type to objects and array expressions (merging the types of all their members), and then propagating those types throughout the analysis (e.g., if `ref` has type `Ref`, then `{ x: ref }` and `[ref]` have type `Structural(value=Ref)` and `{x: ref}.anything` and `[ref][anything]` have type `Ref`).

This allows us to support structures that contain refs, and functions that operate over them, being created and passed around during rendering without at runtime accessing a ref value.

The analysis here uses a fixpoint to allow types to be fully propagated through the system, and we defend against diverging by widening the type of a variable if it could grow infinitely: so, in something like
```
let x = ref;
while (condition) {
  x = [x]
}
```
we end up giving `x` the type `Structural(value=Ref)`.

[ghstack-poisoned]
2024-09-07 17:49:44 -07:00
Joe Savona 727b361528 Fix useMemoCache with setState in render
Fixes the bug that @alexmckenley and @mofeiZ found where setState-in-render can reset useMemoCache and cause an infinite loop. The bug was that renderWithHooksAgain() was not resetting hook state when rerendering (so useMemo values were preserved) but was resetting the updateQueue. This meant that the entire memo cache was cleared on a setState-in-render.

The fix here is to call a new helper function to clear the update queue. It nulls out other properties, but for memoCache it just sets the index back to zero.

ghstack-source-id: fc0947ce21
Pull Request resolved: https://github.com/facebook/react/pull/30889
2024-09-06 14:32:18 -07:00
Mofei ZhangandMofei Zhang bd788b4180 [compiler] Add enablePropagateDepsInHIR flag
Adding new feature flag in preparation for #30894

ghstack-source-id: 59278028cf
Pull Request resolved: https://github.com/facebook/react/pull/30893
2024-09-06 13:02:08 -04:00
Mofei ZhangandMofei Zhang 7b98a168fd [compiler][cleanup] Delete now-unused reactive scope fork
Followup to #30891

ghstack-source-id: 6b42055b5d
Pull Request resolved: https://github.com/facebook/react/pull/30892
2024-09-06 13:02:08 -04:00
Mofei ZhangandMofei Zhang 43264a61d0 [compiler][cleanup] Remove unused enableReactiveScopesInHIR flag
Reactive scopes in HIR has been stable for over 3 months now and is the future direction of react compiler, removing this flag to reduce implementation forks.

ghstack-source-id: 65cdf63cf7
Pull Request resolved: https://github.com/facebook/react/pull/30891
2024-09-06 13:02:08 -04:00
Sebastian MarkbågeandGitHub a03254bc60 [Fiber] Move runWithFiberInDEV from CommitWork to CommitEffects (#30882)
Stacked on #30881.

Move `runWithFiberInDEV` from the recursive part of the commit phase and
instead wrap each call into user space. These should really map 1:1 with
where we're using `try/catch` since that's where we're calling into user
space.

The goal of this is to avoid the extra stack frames added by
`enableOwnerStacks` in the recursive parts to avoid stack overflow. This
way we only have a couple of extra at the end of the stack instead of a
couple of extra at every depth of the tree.
2024-09-05 20:54:39 -04:00
Sebastian MarkbågeandGitHub fe03c56d1e [Fiber] Extract Functions that Call User Space and Host Configs in Commit to Separate Modules (#30881)
This is mostly just moves and same code extracted into utility
functions.

This is to help clarify what needs to be wrapped in try/catch and
runWithFiberInDEV. I'll do the runWithFiberInDEV changes in a follow up.

This leaves ReactCommitWork mostly to do matching on the tag and the
recursive loops.
2024-09-05 20:53:27 -04:00
Sebastian MarkbågeandGitHub a06cd9e1d1 [DevTools] Refactor Forcing Fallback / Error of Suspense / Error Boundaries (#30870)
First, this basically reverts
https://github.com/facebook/react/pull/30517/commits/1f3892ef8cc181218587ddc6accd994890c92ef5
to use a Map/Set to track what is forced to suspend/error again instead
of flags on the Instance. The difference is that now the key in the
Fiber itself instead of the ID. Critically this avoids the
fiberToFiberInstance map to look up whether or not a Fiber should be
forced to suspend when asked by the renderer.

This also allows us to force suspend/error on filtered instances. It's a
bit unclear what should happen when you try to Suspend or Error a child
but its parent boundary is filtered. It was also inconsistent between
Suspense and Error due to how they were implemented.

I think conceptually you're trying to simulate what would happen if that
Component errored or suspended so it would be misleading if we triggered
a different boundary than would happen in real life. So I think we
should trigger the nearest unfiltered Fiber, not the nearest Instance.
The consequence of this however is that if this instance was filtered,
there's no way to undo it without refreshing or removing the filter.
This is an edge case though since it's unusual you'd filter these in the
first place.

It used to be that Suspense walked the store in the frontend and Error
walked the Fibers in the backend. They also did this somewhat eagerly.
This simplifies and unifies the model by passing the id of what you
clicked in the frontend and then we walk the Fiber tree from there in
the backend to lazily find the boundary. However I also eagerly walk the
tree at first to find whether we have any Suspense or Error boundary
parents at all so we can hide the buttons if not.

This also implements it to work with VirtualInstances using #30865. I
find the nearest Fiber Instance downwards filtered or otherwise. Then
from its parent we find the nearest Error or Suspense boundary. That's
because VirtualInstance will always have their inner Fiber as an
Instance but they might not have their parent since it might be
filtered. Which would potentially cause us to skip over a filtered
parent Suspense boundary.
2024-09-05 15:48:17 -04:00
mofeiZandGitHub d72e477814 [compiler runtime] repro: infinite render with useMemoCache + render phase updates (#30849)
Repro for an infinite render bug we found when testing internally. See
equivalent codesandbox repro
[here](https://codesandbox.io/p/sandbox/epic-euclid-mr7lm3).

When render phase updates cause a re-render, useMemoCache arrays for the
fiber are
[cleared](https://github.com/facebook/react/blob/main/packages/react-reconciler/src/ReactFiberHooks.js#L819)
and [recreated on every
retry](https://github.com/facebook/react/blob/main/packages/react-reconciler/src/ReactFiberHooks.js#L1223)
while hook state is preserved.

This pattern (queuing re-renders on the current fiber during render) is
perfectly valid. I believe this is a bug as React compiler currently
replaces `useMemo`s with `useMemoCache` calls and inlined instructions,
taking care to preserve existing memoization dependencies. This should
be the identity transform, but runtime implementation differences mean
that uncompiled code behaves as expected (no infinite render) while
compiled code fails to render.
2024-09-05 13:30:34 -04:00
Sebastian MarkbågeandGitHub 4c58fce777 [DevTools] Avoid getFiberIDUnsafe in debug() Helper (#30878)
Avoids looking up id from fiber and instead pass the instance to the
debug() helper.
2024-09-04 20:25:35 -04:00
Sebastian MarkbågeandGitHub 01ae2ddaa9 [DevTools] Include some Filtered Fiber Instances (#30865)
When we filter Fiber Instances where have no way to recover our position
in the Fiber tree. The extreme form of this is if you filter out all the
Fibers and keep only Server Components.

This affects operations that are performed against fibers such as
collecting Host Instances for highlighting or emulating
suspending/erroring.

Conceptually we don't need to add this into the DevToolsInstance tree
because we only need to get to some Fibers from a VirtualInstance. A
Virtual Instance can contain more than one conceptual child Fiber. It
would be easier if we didn't include them in the tree on one hand
because we could just traverse the tree and assume it looks like the one
on the frontend. But it's also tricky to manage the lifetime. So I went
with a special FilteredFiberInstance node in the tree.

Currently I only add it if its parent would've been a VirtualInstance
since we don't need it in any other cases. If the parent was another
FiberInstance it already has a Fiber.

There might be need for always tracking all Instances whether they're
filtered or not or just moving filtering to the frontend but for now I'm
keeping the general architecture as is.
2024-09-04 19:35:28 -04:00
Joe Savona f820f5a8b6 [compiler] Type inference for tagged template literals
At Meta we have a pattern of using tagged template literals for features that are compiled away:

```
// Relay:
graphql`...graphql text...`
```

In many cases these tags produce a primitive value, and we can get even more optimal output if we can tell the compiler about these types. The new moduleTypeProvider gives us the ability to declare such types, this PR extends the compiler to use this type information for TaggedTemplateExpression values.

ghstack-source-id: 3cd6511b7f
Pull Request resolved: https://github.com/facebook/react/pull/30869
2024-09-04 13:28:33 -07:00
Sebastian MarkbågeandGitHub 0123d7c19f [DevTools] Track root instances in a root Map (#30875)
The FiberRoot is a stateful node that can be tracked this way.

This is another step that will let us remove the
`fiberToFiberInstanceMap`.
2024-09-04 16:15:53 -04:00
Andrew ClarkandGitHub e10e868182 Schedule prerender after something suspends (#30800)
Adds the concept of a "prerender". These special renders are spawned
whenever something suspends (and we're not already prerendering).

The purpose is to move speculative rendering work into a separate phase
that does not block the UI from updating. For example, during a
transition, if something suspends, we should not speculatively prerender
siblings that will be replaced by a fallback in the UI until *after* the
fallback has been shown to the user.
2024-09-04 13:55:29 -04:00
Andrew ClarkandGitHub 8b4c54c00f Temporarily disable suspending during work loop (#30762)
### Based on

- #30761 
- #30759 

---

`use` has an optimization where in some cases it can suspend the work
loop during the render phase until the data has resolved, rather than
unwind the stack and lose context. However, the current implementation
is not compatible with sibling prerendering. So I've temporarily
disabled it until the sibling prerendering has been refactored. We will
add it back in a later step.
2024-09-04 12:38:36 -04:00
Sebastian MarkbågeandGitHub d1afcb43fd [DevTools] Track all public HostInstances in a Map (#30831)
This lets us get from a HostInstance to the nearest DevToolsInstance
without relying on `findFiberByHostInstance` and
`fiberToDevToolsInstanceMap`. We already did the equivalent of this for
Resources in HostHoistables.

One issue before was that we'd ideally get away from the
`fiberToDevToolsInstanceMap` map in general since we should ideally not
treat Fibers as stateful but they could be replaced by something else
stateful in principle.

This PR also addresses Virtual Instances. Now you can select a DOM node
and have it select a Virtual Instance if that's the nearest parent since
the parent doesn't have to be a Fiber anymore.

However, the other reason for this change is that I'd like to get rid of
the need for the `findFiberByHostInstance` from being injected. A
renderer should not need to store a reference back from its instance to
a Fiber. Without the Synthetic Event system this wouldn't be needed by
the renderer so we should be able to remove it. We also don't really
need it since we have all the information by just walking the commit to
collect the nodes if we just maintain our own Map.

There's one subtle nuance that the different renderers do. Typically a
HostInstance is the same thing as a PublicInstance in React but
technically in Fabric they're not the same. So we need to translate
between PublicInstance and HostInstance. I just hardcoded the Fabric
implementation of this since it's the only known one that does this but
could feature detect other ones too if necessary. On one hand it's more
resilient to refactors to not rely on injected helpers and on hand it
doesn't follow changes to things like this.

For the conflict resolution I added in #30494 I had to make that
specific to DOM so we can move the DOM traversal to the backend instead
of the injected helper.
2024-09-03 17:28:05 -04:00
Sebastian MarkbågeandGitHub 8d68da3f73 [Fiber] Stash ThenableState on the Dependencies Object for Use By DevTools (#30866)
This lets us track what a Component might suspend on from DevTools. We
could already collect this by replaying a component's Hooks but that
would be expensive to collect from a whole tree.

The thenables themselves might contain useful information but mainly
we'd want access to the `_debugInfo` on the thenables which might
contain additional information from the server.


https://github.com/facebook/react/blob/19bd26beb689e554fceb0b929dc5199be8cba594/packages/shared/ReactTypes.js#L114

In a follow up we should really do something similar in Flight to
transfer `use()` on the debugInfo of that Server Component.
2024-09-03 16:04:24 -04:00
Sebastian MarkbågeandGitHub e0a07e9738 [DevTools] Support VirtualInstances in findAllCurrentHostInstances (#30853)
This lets us highlight Server Components.

However, there is a problem with this because if the actual nearest
Fiber is filtered, there's no FiberInstance and so we might skip past it
and maybe never find a child while walking the whole tree. This is very
common in the case where you have just Server Components and Host
Components which are filtered by default.

Note how the DOM nodes that are just plain host instances without client
component wrappers are not highlighted here:

<img width="1102" alt="Screenshot 2024-08-30 at 4 33 55 PM"
src="https://github.com/user-attachments/assets/c9a7b91e-5faf-4c60-99a8-1195539ff8b5">

Fixing that needs a separate refactor though and related to several
other features that already have a similar issue without
VirtualInstances like Suspense/Error Boundaries (triggering
suspense/error on a filtered Suspense/ErrorBoundary doesn't work
correctly). So this first PR just adds the feature for the common case
where there's at least some Fibers.
2024-09-03 12:29:59 -04:00
Sebastian MarkbågeandGitHub 04ec50efa9 [DevTools] Add Filtering of Environment Names (#30850)
Stacked on #30842.

This adds a filter to be able to exclude Components from a certain
environment. Default to Client or Server.

The available options are computed into a dropdown based on the names
that are currently used on the page (or an option that were previously
used). In addition to the hardcoded "Client". Meaning that if you have
Server Components on the page you see "Server" or "Client" as possible
options but it can be anything if there are multiple RSC environments on
the page.

"Client" in this case means Function and Class Components in Fiber -
excluding built-ins.

If a Server Component has two environments (primary and secondary) then
both have to be filtered to exclude it.

We don't show the option at all if there are no Server Components used
in the page to avoid confusing existing users that are just using Client
Components and wouldn't know the difference between Server vs Client.

<img width="815" alt="Screenshot 2024-08-30 at 12 56 42 AM"
src="https://github.com/user-attachments/assets/e06b225a-e85d-4cdc-8707-d4630fede19e">
2024-09-03 12:29:15 -04:00
Sebastian MarkbågeandGitHub 4f60494156 [Flight] Ship DEV-only enableServerComponentLogs flag in Stable/Canary (#30847)
To recap. This only affects DEV and RSC. It patches console on the
server in DEV (similar to how React DevTools already does and what we
did for the double logging). Then replays those logs with a `[Server]`
badge on the client so you don't need a server terminal open.

This has been on for over 6 months now in our experimental channel and
we've had a lot of coverage in Next.js due to various experimental flags
like taint and ppr.

It's non-invasive in that even if something throws we just serialize
that as an unknown value.

The main feedback we've gotten was:

- The serialization depth wasn't deep enough which I addressed in #30294
and haven't really had any issues since. This could still be an issue or
the inverse that you serialize too many logs that are also too deep.
This is not so much an issue with intentional logging and things like
accidental errors don't typically have unbounded arguments (e.g. React
errors are always string arguments). The ideal would be some way to
retain objects and then load them on-demand but that needs more
plumbing. Which can be later.
- The other was that double logging on the server is annoying if the
same terminal does both the RSC render and SSR render which was
addressed in #30207. It is now off by default in node/edge-builds of the
client, on by default in browser builds. With the `replayConsole` option
to either opt-in or out.

We've reached a good spot now I think.

These are better with `enableOwnerStacks` but that's a separate track
and not needed.

The only thing to document here, other than maybe that we're doing it,
is the `replayConsole` option but that's part of the RSC renderers that
themselves are not documented so nowhere to document it.
2024-08-30 15:11:57 -04:00
Sebastian MarkbågeandGitHub e56f4ae38d [DevTools] Support secondary environment name when it changes (#30842)
We currently support the Environment Name change within a Component.
#29867

If this happens, we give it two HoCs. The problem with this is that we
only show one followed by `+1` in the list.

Before:
<img width="529" alt="Screenshot 2024-08-28 at 6 50 31 PM"
src="https://github.com/user-attachments/assets/c080be72-c254-4d4d-89b6-d1b7f9a9ada8">

After:
<img width="1101" alt="Screenshot 2024-08-28 at 7 16 21 PM"
src="https://github.com/user-attachments/assets/04718674-164b-4255-9cf6-dec9198f12b7">

I could potentially instead badge this case as `A/B` in a single badge.
2024-08-30 10:05:19 -04:00
Ruslan LesiutinandGitHub 8308d2f1fe fix[react-devtools/ReactDebugHooks]: support unstable prefixes in hooks and useContextWithBailout (#30837)
Related - https://github.com/facebook/react/pull/30407.

This is experimental-only and FB-only hook. Without these changes,
inspecting an element that is using this hook will throw an error,
because this hook is missing in Dispatcher implementation from React
DevTools, which overrides the original one to build the hook tree.

![Screenshot 2024-08-28 at 18 42
55](https://github.com/user-attachments/assets/e3bccb92-74fb-4e4a-8181-03d13f8512c0)

One nice thing from it is that in case of any potential regressions
related to this experiment, we can quickly triage which implementation
of `useContext` is used by inspecting an element in React DevTools.

Ideally, I should've added some component that is using this hook to
`react-devtools-shell`, so it can be manually tested, but I can't do it
without rewriting the infra for it. This is because this hook is only
available from fb-www builds, and not experimental.
2024-08-30 10:34:52 +01:00
Rune BottenandGitHub 394e75d9a9 [DevTools] Increase max payload for websocket in standalone app (#30848)
<!--
  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

<!--
Explain the **motivation** for making this change. What existing problem
does the pull request solve?
-->

When debugging applications that are experiencing runaway re-rendering,
it is helpful to profile them in the React Developer Tools.
Unfortunately there is a size limit on the captured profile which can
make them impossible to inspect or save. The limitations I have found
are in `postMessage` for the Chrome extension and in the `ws` websocket
server for the standalone app.

Profiling an app that produces a large profile artifact will simply show
that no profiling data was captured and output an error in the console,
here shown for the standalone app:

```text
standalone.js:92 [React DevTools] Error with websocket connection i {target: H, type: 'error', message: 'Max payload size exceeded', error: RangeError: Max payload size exceeded
    at e.exports.haveLength (/Users/rune/.npm/_npx/8ea6ac5c50…}error: RangeError: Max payload size exceeded
```

This change simply increases the max payload of the websocket server in
the standalone app so that larger profiles may be captured and
inspected.

## How did you test this change?

<!--
Demonstrate the code is solid. Example: The exact commands you ran and
their output, screenshots / videos if the pull request changes the user
interface.
How exactly did you verify that your PR solves the issue you wanted to
solve?
  If you leave this empty, your PR will very likely be closed.
-->

I verified that I could capture and inspect profiling data that
previously exceeded the default limitation for a particular app
2024-08-30 10:34:27 +01:00
Joe Savona 071dd00366 [compiler] Errors in earlier functions dont stop subsequent compilation
Errors in an earlier component/hook shouldn't stop later components from compiling.

ghstack-source-id: 6e04a5bb2e
Pull Request resolved: https://github.com/facebook/react/pull/30844
2024-08-29 22:41:53 -07:00
Sebastian MarkbågeandGitHub 61739a8a0f [DevTools] Filter Server Components (#30839)
Support filtering Virtual Instances with existing filters.

Server Components are considered "Functions".

In a follow up I'll a new filter for "Environment" which will let you
filter by Client vs Server (and more).
2024-08-29 12:49:30 -04:00
Sebastian MarkbågeandGitHub e33a7233a7 [DevTools] Track virtual instances on the tracked path for selections (#30802)
This appends a (filtered) virtual instance path at the end of the fiber
path. If a virtual instance is selected inside the fiber.

The main part of the path is still just the fiber path since that's the
semantically stateful part. Then we just tack on a few virtual path
frames at the end if we're currently selecting a specific Server
Component within the nearest Fiber.

I also took the opportunity to fix a bug which caused selections inside
Suspense boundaries to not be tracked.
2024-08-29 12:45:03 -04:00
Sebastian MarkbågeandGitHub 18bf7bf500 [DevTools] Remove displayName from inspected data (#30841)
This just clarifies that this is actually unused in the front end. We
use the name from the original instance as the canonical name.
2024-08-29 12:44:48 -04:00
Ruslan LesiutinandGitHub a19a8ab44f chore[react-devtools/hook]: remove unused native values (#30827)
Stacked on https://github.com/facebook/react/pull/30826. See [this
commit](https://github.com/facebook/react/pull/30827/commits/ec0e48ed7a47dbbdafb5e2530ccba1f2e5b17bad).

This is unused.
2024-08-29 11:34:31 +01:00
Ruslan LesiutinandGitHub 233d63c497 chore[react-devtools/extensions]: remove unused storage permission (#30826)
Stacked on https://github.com/facebook/react/pull/30825. See [this
commit](https://github.com/facebook/react/pull/30826/commits/b2130701cf6b25d7a96c1e92b44f41affa56bb35).

We are not using `storage` anywhere yet, but will be soon. This
permission is not needed.
2024-08-29 11:32:18 +01:00
Ruslan LesiutinandGitHub 795b3207ce fix[react-devtools/extensions]: fixed tabs API calls and displaying restricted access popup (#30825)
Stacked on https://github.com/facebook/react/pull/30824. See [this
commit](https://github.com/facebook/react/pull/30825/commits/c9830d64749cf8fd592ea30a1cd65842cf83f6df).

Turns out we should be listing `tabs` in our permissions, if we want to
be able to receive tab url, once its updated.
This also fixes `chrome.tabs.onCreated` event subscription, because [it
should receive only tab
object](https://developer.chrome.com/docs/extensions/reference/api/tabs#event-onCreated),
and not 3 arguments, as expected in the previous implementation.
2024-08-29 11:31:43 +01:00
Ruslan LesiutinandGitHub 537c74e16a feat[react-devtools]: support Manifest v3 for Firefox extension (#30824)
Firefox [finally supports
`ExecutionWorld.MAIN`](https://bugzilla.mozilla.org/show_bug.cgi?id=1736575)
in content scripts, which means we can migrate the browser extension to
Manifest V3.

This PR also removes a bunch of no longer required explicit branching
for Firefox case, when we are using Manifest V3-only APIs.

We are also removing XMLHttpRequest injection, which is no longer needed
and restricted in Manifest V3. The new standardized approach (same as in
Chromium) doesn't violate CSP rules, which means that extension can
finally be used for apps running in production mode.
2024-08-29 11:28:35 +01:00
Joe Savona fc0df475c4 [compiler] Inferred deps must match exact optionality of manual deps
To prevent any difference in behavior, we check that the optionality of the inferred deps exactly matches the optionality of the manual dependencies. This required a fix, I was incorrectly inferring optionality of manual deps (they're only optional if OptionalTerminal.optional is true) - for nested cases of mixed optional/non-optional.

ghstack-source-id: afd49e89cc
Pull Request resolved: https://github.com/facebook/react/pull/30840
2024-08-28 15:59:26 -07:00
Joe Savona 3a45ba241c [compiler] Enable optional dependencies by default
Per title. This gives us much more granular memoization when the source used optional member expressions. Note that we only infer optional deps when the source used optionals: we don't (yet) infer optional dependencies from conditionals.

ghstack-source-id: 104d0b712d
Pull Request resolved: https://github.com/facebook/react/pull/30838
2024-08-28 15:59:26 -07:00
Joe Savona 99a4b26e18 [compiler] Handle optional where innermost property access is non-optional
Handles an additional case as part of testing combinations of the same path being accessed in different places with different segments as optional/unconditional.

ghstack-source-id: ace777fcbb
Pull Request resolved: https://github.com/facebook/react/pull/30836
2024-08-28 15:59:25 -07:00
Joe Savona 7475d568da [wip][compiler] Infer optional dependencies
Updates PropagateScopeDeps and DeriveMinimalDeps to understand optional dependency paths (`a?.b`). There a few key pieces to this:

In PropagateScopeDeps we jump through some hoops to work around the awkward structure of nested OptionalExpressions. This is much easier in HIR form, but I managed to get this pretty close and i think it will be landable with further cleanup. A good chunk of this is avoiding prematurely registering a value as a dependency - there are a bunch of indirections in the ReactiveFunction structure:

```
t0 = OptionalExpression
  SequenceExpression
    t0 = Sequence
      ...
    LoadLocal t0
```

Where if at any point we call `visitOperand()` we'll prematurely register a dependency instead of declareProperty(). The other bit is that optionals can be optional=false for nested member expressions where not all the parts are actually optional (`foo.bar?.bar.call()`). And of course, parts of an optional chain can still be conditional even when optional=true (for example the `x` in `foo.bar?.[x]?.baz`). Not all of this is tested yet so there are likely bugs still.

The other bit is DeriveMinimalDeps, which is thankfully easier. We add OptionalAccess and OptionalDep and update the merge and reducing logic for these cases. There is probably still more to update though, for things like merging subtrees. There are a lot of ternaries that assume a result can be exactly one of two states (conditional/unconditional, dependency/access) and these assumptions don't hold anymore. I'd like to refactor to dependency/access separate from conditional/optional/unconditional. Also, the reducing logic isn't quite right: once a child is optional we keep inferring all the parents as optional too, losing some precision. I need to adjust the reducing logic to let children decide whether their path token is optional or not.

ghstack-source-id: 207842ac64
Pull Request resolved: https://github.com/facebook/react/pull/30819
2024-08-28 15:59:25 -07:00
Joe Savona 9180a37fba [compiler] Allow inferred non-optional paths when manual deps were optional
If the inferred deps are more precise (non-optional) than the manual deps (optional) it should pass validation.

The other direction also seems like it would be fine - inferring optional deps when the original was non-optional - but for now let's keep the "at least as precise" rule.

ghstack-source-id: 9f7a99ee5f
Pull Request resolved: https://github.com/facebook/react/pull/30816
2024-08-28 15:59:25 -07:00
Joe Savona 925c20a206 [compiler] Add fallthrough to branch terminal
Branch terminals didn't have a fallthrough because they correspond to an outer terminal (optional, logical, etc) that has the "real" fallthrough. But understanding how branch terminals correspond to these outer terminals requires knowing the branch fallthrough. For example, `foo?.bar?.baz` creates terminals along the lines of:

```
bb0:
  optional fallthrough=bb4
bb1:
  optional fallthrough=bb3
bb2:
  ...
  branch ... (fallthrough=bb3)

...

bb3:
  ...
  branch ... (fallthrough=bb4)

...

bb4:
  ...
```

Without a fallthrough on `branch` terminals, it's unclear that the optional from bb0 has its branch node in bb3. With the fallthroughs, we can see look for a branch with the same fallthrough as the outer optional terminal to match them up.

ghstack-source-id: d48c623289
Pull Request resolved: https://github.com/facebook/react/pull/30814
2024-08-28 15:59:25 -07:00
Joe Savona a718da0b23 [compiler] Add DependencyPath optional property
Adds an `optional: boolean` property to each token in a DependencyPath, currently always set to false. Also updates the equality and printing logic for paths to account for this field.

Subsequent PRs will update our logic to determine which manual dependencies were optional, then we can start inferring optional deps as well.

ghstack-source-id: 66c2da2cfa
Pull Request resolved: https://github.com/facebook/react/pull/30813
2024-08-28 15:59:25 -07:00
Joe Savona 4759161ed8 [compiler] Wrap ReactiveScopeDep path tokens in object
Previously the path of a ReactiveScopeDependency was `Array<string>`. We need to track whether each property access is optional or not, so as a first step we change this to `Array<{property: string}>`, making space for an additional property in a subsequent PR.

ghstack-source-id: c5d38d72f6
Pull Request resolved: https://github.com/facebook/react/pull/30812
2024-08-28 15:59:25 -07:00
Joe Savona 5e51d767d1 [compiler] Stop reusing ScopeDep type in AnalyzeFunctions
AnalyzeFunctions was reusing the `ReactiveScopeDependency` type since it happened to have a convenient shape, but we need to change this type to represent optionality. We now use a locally defined type instead.

ghstack-source-id: e305c6ede4
Pull Request resolved: https://github.com/facebook/react/pull/30811
2024-08-28 15:59:25 -07:00
Mike Vitousek 7771d3a797 [compiler] Track refs through object expressions and property lookups
Summary:
This addresses the issue of the compiler being overly restrictive about refs escaping into object expressions. Rather than erroring whenever a ref flows into an object, we will now treat the object itself as a ref, and apply the same escape rules to it. Whenever we look up a property from a ref value, we now don't know whether that value is itself a ref or a ref value, so we assume it's both.

The same logic applies to ref-accessing functions--if such a function is stored in an object, we'll propagate that property to the object itself and any properties looked up from it.

ghstack-source-id: 5c6fcb895d
Pull Request resolved: https://github.com/facebook/react/pull/30821
2024-08-27 10:11:50 -07:00
Mike Vitousek f2841c2a49 [compiler] Fixture to demonstrate issue with returning object containing ref
Summary:
We currently can return a ref from a hook but not an object containing a ref.

ghstack-source-id: 8b1de4991e
Pull Request resolved: https://github.com/facebook/react/pull/30820
2024-08-27 10:11:50 -07:00
Sebastian MarkbågeandGitHub 96aca5f4f3 Spawn new task if we hit stack overflow (#30419)
If we see the "Maximum call stack size exceeded" error we know we've hit
stack overflow. We can recover from this by spawning a new task and
trying again. Effectively a zero-cost trampoline in the normal case. The
new task will have a clean stack. If you have a lot of siblings at the
same depth that hits the limit you can end up hitting this once for each
sibling but within that new sibling you're unlikely to hit this again.
So it's not too expensive.

If it errors again in the retryTask pass, the other error handling takes
over which causes this to be able to still not infinitely stall. E.g.
when the component itself throws an error like this.

It's still better to increase the stack limit for performance if you
have a really deep tree but it doesn't really hurt to be able to recover
since it's zero cost when it doesn't happen.

We could do the same thing for Flight. Those trees don't tend to be as
deep but could happen.
2024-08-27 13:10:37 -04:00
Sebastian MarkbågeandGitHub f90a6bcc4c [DevTools] Reconcile Fibers Against Previous Children Instances (#30822)
This loops over the remainingReconcilingChildren to find existing
FiberInstances that match the updated Fiber. This is the same thing we
already do for virtual instances. This avoids the need for a
`fiberToFiberInstanceMap`.

This loop is fast but there is a downside when the children set is very
large and gets reordered with keys since we might have to loop over the
set multiple times to get to the instances in the bottom. If that
becomes a problem we can optimize it the same way ReactChildFiber does
which is to create a temporary Map only when the children don't line up
properly. That way everything except the first pass can use the Map but
there's no need to create it eagerly.

Now that we have the loop we don't need the previousSibling field so we
can save some memory there.
2024-08-27 12:05:47 -04:00
Sebastian MarkbågeandGitHub 9690b9ad74 [DevTools] Remove findCurrentFiberUsingSlowPathByFiberInstance (#30818)
We always track the last committed Fiber on `FiberInstance.data`.


https://github.com/facebook/react/blob/dcae56f8b72f625d8affe5729ca9991b31a492ac/packages/react-devtools-shared/src/backend/fiber/renderer.js#L3068

So we can now remove this complex slow path to get the current fiber.
2024-08-27 12:05:24 -04:00
Sebastian MarkbågeandGitHub 1a8f92a869 [DevTools] Track Tree Base Duration of Virtual Instances (#30817)
These don't have their own time since they don't take up any time to
render but they show up in the tree for context. However they never
render themselves. Their base tree time is the base time of their
children. This way they take up the same space as their combined
children in the Profiler tree. (Instead of leaving a blank line which
they did before this PR.)

The frontend doesn't track the difference between a virtual instance and
a Fiber that didn't render this update. This might be a bit confusing as
to why it didn't render. I add the word "client" to make it a bit
clearer and works for both. We should probably have different verbiage
here based on it is a Server Component or something else.

<img width="1103" alt="Screenshot 2024-08-26 at 5 00 47 PM"
src="https://github.com/user-attachments/assets/87b811d4-7024-466a-845d-542493ed3ca2">

I also took the opportunity to remove idToTreeBaseDurationMap and
idToRootMap maps. Cloning the Map isn't really all that super fast
anyway and it means we have to maintain the map continuously as we
render. Instead, we can track it on the instances and then walk the
instances to create a snapshot when starting to profile. This isn't as
fast but really fast too and requires less bookkeeping while rendering
instead which is more sensitive than that one snapshot in the beginning.
2024-08-27 12:05:10 -04:00
Benoit GirardandGitHub 246d7bfeb0 Enable suspenseCallback on React Native (#29210)
## Summary

suspenseCallback feature has proved to be useful for FB Web. Let's look
at enabling the feature for the React Native build.


## How did you test this change?

Will sync the react changes with a React Native build and will verify
that performance logging is correctly notified of suspense promises
during the suspense callback.
2024-08-26 22:06:02 -04:00
Sebastian MarkbågeandGitHub e44685e4f1 [DevTools] Use Owner Stacks to Implement View Source of a Server Component (#30798)
We don't have the source location of Server Components on the client
because we don't want to eagerly do the throw trick for all Server
Components just in case. Unfortunately Node.js doesn't expose V8's API
to get a source location of a function.

We do have the owner stacks of the JSX that created it though and at
some point we'll also show that location in DevTools.

However, the realization is that if a Server Component is the owner of
any child. The owner stack of that child will have the owner component's
source location as its bottom stack frame.

The technique I'm implementing here is to track whenever a child mounts
we already have its owner. We track the first discovered owned child's
stack on the owner. Then when we ask for a Source location of the owner
do we parse that stack and extract the location of the bottom frame.
This doesn't give us a location necessarily in the top of the function
but somewhere in the function.

In this case the first owned child is the Container:

<img width="1107" alt="Screenshot 2024-08-22 at 10 24 42 PM"
src="https://github.com/user-attachments/assets/95f32850-24a5-4151-8ce6-b7b89db68aee">
<img width="648" alt="Screenshot 2024-08-22 at 10 24 20 PM"
src="https://github.com/user-attachments/assets/4bcba033-866f-4684-9beb-de09d189deff">

We can even use this technique for Fibers too. Currently I use this as a
fallback in case the error technique didn't work. This covers a case
where nothing errors but you still render a child. This case is actually
quite common:

```
function Foo() {
  return <Bar />;
}
```

However, for Fibers we could really just use the `inspect(function)`
technique which works for all cases. At least in Chrome.

Unfortunately, this technique doesn't work if a Component doesn't create
any new JSX but just renders its children. It also currently doesn't
work if the child is filtered since I only look up the owner if an
instance is not filtered. This means that the container in the fixture
can't view source by default since the host component is filtered:

```
export default function Container({children}) {
  return <div>{children}</div>;
}
```

<img width="1107" alt="Screenshot 2024-08-22 at 10 24 35 PM"
src="https://github.com/user-attachments/assets/c3f8f9c5-5add-4d35-9290-3a5079e82adc">
2024-08-26 20:50:43 -04:00
Lauren Tan dcae56f8b7 [ez] Remove trailing space from babel-refresh header
ghstack-source-id: d78c53462b
Pull Request resolved: https://github.com/facebook/react/pull/30806
2024-08-26 13:16:03 -04:00
Sebastian MarkbågeandGitHub f65ac7bd4a [DevTools] Make function inspection instant (#30786)
I noticed that there is a delay due to the inspection being split into
one part that gets the attribute and another eval that does the
inspection. This is a bit hacky and uses temporary global names that are
leaky. The timeout was presumably to ensure that the first step had
fully propagated but it's slow. As we've learned, it can be throttled,
and it isn't a guarantee either way.

Instead, we can just consolidate these into a single operation that
by-passes the bridge and goes straight to the renderer interface from
the eval.

I did the same for the viewElementSource helper even though that's not
currently in use since #28471 but I think we probably should return to
that technique when it's available since it's more reliable than the
throw - at least in Chrome. I'm not sure about the status of React
Native here. In Firefox, inspecting a function with source maps doesn't
seem to work. It doesn't jump to original code.
2024-08-26 11:53:17 -04:00