Commit Graph

19428 Commits

Author SHA1 Message Date
Jan Kassens
d3d4d3a46b Call cleanup of insertion effects when hidden (#30954)
Insertion effects do not unmount when a subtree is removed while
offscreen.

Current behavior for an insertion effect is if the component goes

- *visible -> removed:* calls insertion effect cleanup
- *visible -> offscreen -> removed:* insertion effect cleanup is never
called

This makes it so we always call insertion effect cleanup when removing
the component.

Likely also fixes https://github.com/facebook/react/issues/26670

---------

Co-authored-by: Rick Hanlon <rickhanlonii@fb.com>
2024-09-13 16:18:14 -04:00
Mike Vitousek
633a0fe536 [compiler] Factor out function effects from reference effects
Summary:
This PR performs a major refactor of InferReferenceEffects to separate out the work on marking places with Effects from inferring FunctionEffects. The behavior should be identical after this change (see [internal sync](https://www.internalfb.com/intern/everpaste/?handle=GN74VxscnUaztTYDAL8q0CRWBIxibsIXAAAB)) but the FunctionEffect logic should be easier to work with.

These analyses are unfortunately still deeply linked--the FunctionEffect analysis needs to reason about the "current" value kind for each point in the program, while the InferReferenceEffects algorithm performs global updates on the state of the program (e.g. freezing). In the future, it might be possible to make these entirely separate passes if we store the ValueKind directly on places.

For the most part, the logic of reference effects and function effects can be cleanly separated: for each instruction and terminal, we visit its places and infer their effects, and then we visit its places and infer any function effects that they cause. The biggest wrinkle here is that when a transitive function freeze operation occurs, it has to happen *after* inferring the function effects on the place, because otherwise we may convert a value from Context to Frozen, which will cause the ContextualMutation function effect to be converted to a ReactMutation effect too early. This can be observed in a case like this:

```
export default component C() {
  foo(() => {
    const p = {};
    return () => {
      p['a'] = 1
    };
  });
}
```
Here when the outer function returns the inner function, it freezes the inner function which transitively freezes `p`. But before that freeze happens, we need to replay the ContextualMutation on the inner function to determine that the value is mutable in the outer context. If we froze `p` first, we would instead convert the ContextualMutation to a ReactMutation and error.

To handle this, InferReferenceEffects now delays the exection of the freezeValue action until after it's called the helper functions that generate function effects. So the order of operations on a given place is now

set effect --> generate function effects --> transitively freeze dependencies, if applicable

ghstack-source-id: 21cb50c140
Pull Request resolved: https://github.com/facebook/react/pull/30920
2024-09-13 12:38:17 -07:00
Sebastian Markbåge
94e4acaa14 [Fiber] Set profiler values to doubles (#30942)
At some point this trick was added to initialize the value first to NaN
and then replace them with zeros and negative ones.

This is to address the issue noted in
https://github.com/facebook/react/issues/14365 where these hidden
classes can be initialized to SMIs at first and then deopt when we
realize they're actually doubles.

However, this fix has been long broken and has deopted the profiling
build for years because closure compiler optimizes out the first write.

I'm not sure because I haven't A/B-tested this in the JIT yet but I
think we can use negative zero and -1.1 as the initial values instead
since they're not simple integers. Negative zero `===` zero (but not
Object.is) so is the same as far as our code is concerned. The negative
value is just `< 0` comparisons.
2024-09-13 12:27:00 -04:00
Mofei Zhang
206df66e70 [compiler][rewrite] PropagateScopeDeps hir rewrite
Resubmission of #30079 -- core logic unchanged, but needed to rebase past #30573

### Quick background
#### Temporaries

The compiler currently treats temporaries and named variables (e.g. `x`) differently in this pass.
- named variables may be reassigned (in fact, since we're running after LeaveSSA, a single named identifier's IdentifierId may map to multiple `Identifier` instances -- each with its own scope and mutable range)
- temporaries are replaced with their represented expressions during codegen. This is correct (mostly correct, see #29878) as we're careful to always lower the correct evaluation semantics. However, since we rewrite reactive scopes entirely (to if/else blocks), we need to track temporaries that a scope produces in `ReactiveScope.declarations` and later promote them to named variables.
In the same example, $4, $5, and $6 need to be promoted: $2 ->`t0`,  $5 ->`t1`, and $6 ->`t2`.
```js
[1] $2 = LoadGlobal(global) foo
[2] $3 = LoadLocal bar$1
scope 0:
  [3] $4 = Call $2(<unknown> $3)
scope 1:
  [4] $5 = Object {  }
scope 2:
  [5] $6 = Object { a: $4, b: $5 }
[6] $8 = StoreLocal Const x$7 = $6
```

#### Dependencies
`ReactiveScope.dependencies` records the set of (read-only) values that a reactive scope is dependent on. This is currently limited to just variables (named variables from source and promoted temporaries) and property-loads.
All dependencies we record need to be hoistable -- i.e. reordered to just before the ReactiveScope begins. Not all PropertyLoads are hoistable.

In this example, we should not evaluate `obj.a.b` without before creating x and checking `objIsNull`.
```js
// reduce-reactive-deps/no-uncond.js
function useFoo({ obj, objIsNull }) {
  const x = [];
  if (isFalse(objIsNull)) {
    x.push(obj.a.b);
  }
  return x;
}
```

While other memoization strategies with different constraints exist, the current compiler requires that `ReactiveScope.dependencies` be re-orderable to the beginning of the reactive scope. But.. `PropertyLoad`s from null values will throw `TypeError`. This means that evaluating hoisted dependencies should throw if and only if the source program throws. (It is also a bug if source throws and compiler output does not throw. See https://github.com/facebook/react-forget/pull/2709)

---
### Rough high level overview
1. Pass 1
Walk over instructions to gather every temporary used outside of its defining scope (same as ReactiveFunction version). These determine the sidemaps we produce, as temporaries used outside of their declaring scopes get promoted to named variables later (and are not considered hoistable rvals).
2. Pass 2 (collectTemporariesSidemap)
Walk over instructions to generate a sidemap of temporary identifier -> named variable and property path (e.g. `$3 -> {obj: props, path: ["a", "b"]}`)
2. Pass 2 (collectHoistablePropertyLoads)
  a. Build a sidemap of block -> accessed variables and properties (e.g. `bb0 -> [ {obj: props, path: ["a", "b"]} ]`)
  b. Propagate "non-nullness" i.e. variables and properties for which we can safely evaluate `PropertyLoad`.
  A basic block can unconditionally read from identifier X if any of the following applies:
    - the block itself reads from identifier X
    - all predecessors of the block read from identifier X
    - all successors of the block read from identifier X
4. Pass 3: (collectDependencies)
Walks over instructions again to record dependencies and declarations, using the previously produced sidemaps. We do not record any control-flow here
5. Merge every scope's recorded dependencies with the set of hoistable PropertyLoads

Tested by syncing internally and (1) checking compilation output differences ([internal link](https://www.internalfb.com/intern/everpaste/?handle=GPCfUBt_HCoy_S4EAJDVFJyJJMR0bsIXAAAB)), running internally e2e tests ([internal link](https://fburl.com/sandcastle/cs5mlkxq))

---
### Followups:
1. Rewrite function expression deps
This change produces much more optimal output as the compiler now uses the function CFG to understand which variables / paths are assumed to be non-null. However, it may exacerbate [this function-expr hoisting bug](https://github.com/facebook/react/blob/main/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/bug-invalid-hoisting-functionexpr.tsx). A short term fix here is to simply call some form of `collectNonNullObjects` on every function expression to find hoistable variable / paths. In the longer term, we should refactor out `FunctionExpression.deps`.

2. Enable optional paths
(a) don't count optional load temporaries as dependencies (e.g. `collectOptionalLoadRValues(...)`).
(b) record optional paths in both collectHoistablePropertyLoads and dependency collection

ghstack-source-id: 2507f6ea751dce09ad1dccd353ae6fc7cf411582
Pull Request resolved: https://github.com/facebook/react/pull/30894
2024-09-12 17:40:57 -04:00
Mofei Zhang
5ac4034e14 [compiler] Fork fixtures for enablePropagateDepsInHIR
- flip `enablePropagateDepsInHIR` to off by default
- fork fixtures which produce compilation differences in #30894 to separate directory `propagate-scope-deps-hir-fork`, to be cleaned up when we remove this flag

ghstack-source-id: 7d5b8dc29788a65c272c846af9877b09fbf2cd60
Pull Request resolved: https://github.com/facebook/react/pull/30949
2024-09-12 17:40:57 -04:00
Mofei Zhang
f6dcce5199 [compiler][ez] Add entrypoints to ssa fixtures
Adds evaluator support for a few compiler test fixtures

ghstack-source-id: 202654992a9876cea59885b54a338c908e369ddb
Pull Request resolved: https://github.com/facebook/react/pull/30948
2024-09-12 17:40:57 -04:00
Sebastian Markbåge
dff50825c6 [Flight] Track owner/stack where the Flight Client reads as the root (#30933)
This means that the owner of a Component rendered on the remote server
becomes the Component on this server.

Ideally we'd support this for the Client side too. In particular Fiber
but currently ReactComponentInfo's owner is typed as only supporting
other ReactComponentInfo and it's a bigger lift to support that.
2024-09-12 17:19:34 -04:00
Sebastian Markbåge
89b445709d Enable lazy context propagation (#30935)
Last I heard this was great so not sure there are any more blockers to
just include it in 19?
2024-09-12 13:01:56 -04:00
Josh Story
94e652d505 disable enableSiblingPrerendering in experimental channel (#30952)
Disables `enableSiblingPrerendering` in the experimental builds until
the feature is tested at Meta first.
2024-09-12 09:33:20 -07:00
Sebastian Markbåge
473522093d [Fizz] Add resumeAndPrerender to Static Rendering (#30950)
This is only in the same experimental exports as `resume`. Useful with
Postpone/Halt.

We already have `prerender()` to create a partial tree with postponed
state. We also have `resume()` to dynamically resume such a tree.

This lets you do a new prerender by resuming an already existing
postponed state. Basically creating a chain of preludes. The next
prelude would include the scripts to patch up the document.

This mostly just works since both prerender and resume are already
implemented using the same code so we just enable both at the root. I'm
sure we'll find some edge cases since this wasn't considered when it was
first written but so far I've only found an unrelated existing bug with
`keyPath` fixed here.
2024-09-12 10:51:01 -04:00
Ruslan Lesiutin
bb6b86ed59 refactor[react-devtools]: initialize renderer interface early (#30946)
The current state is that `rendererInterface`, which contains all the
backend logic, like generating component stack or attaching errors to
fibers, or traversing the Fiber tree, ..., is only mounted after the
Frontend is created.

For browser extension, this means that we don't patch console or track
errors and warnings before Chrome DevTools is opened.

With these changes, `rendererInterface` is created right after
`renderer` is injected from React via global hook object (e. g.
`__REACT_DEVTOOLS_GLOBAL_HOOK__.inject(...)`.

Because of the current implementation, in case of multiple Reacts on the
page, all of them will patch the console independently. This will be
fixed in one of the next PRs, where I am moving console patching to the
global Hook.

This change of course makes `hook.js` script bigger, but I think it is a
reasonable trade-off for better DevX. We later can add more heuristics
to optimize the performance (if necessary) of `rendererInterface` for
cases when Frontend was connected late and Backend is attempting to
flush out too many recorded operations.

This essentially reverts https://github.com/facebook/react/pull/26563.
2024-09-12 13:59:29 +01:00
Andrew Clark
d6cb4e7713 Start prerendering Suspense retries immediately (#30934)
When a component suspends and is replaced by a fallback, we should start
prerendering the fallback immediately, even before any new data is
received. During the retry, we can enter prerender mode directly if
we're sure that no new data was received since we last attempted to
render the boundary.

To do this, when completing the fallback, we leave behind a pending
retry lane on the Suspense boundary. Previously we only did this once a
promise resolved, but by assigning a lane during the complete phase, we
will know that there's speculative work to be done.

Then, upon committing the fallback, we mark the retry lane as suspended
— but only if nothing was pinged or updated in the meantime. That allows
us to immediately enter prerender mode (i.e. render without skipping any
siblings) when performing the retry.
2024-09-11 11:41:54 -04:00
Sebastian Markbåge
1bb056363c [Fizz] Use RequestInstance constructor for resuming (#30947)
We added enough fields to need a constructor instead of inline object in
V8.

We didn't update the resumeRequest path though so it wasn't using the
constructor and had a different hidden class.
2024-09-11 11:35:01 -04:00
Ruslan Lesiutin
344bc8128b refactor[Agent/Store]: Store to send messages only after Agent is initialized (#30945)
Both for browser extension, and for React Native (as part of
`react-devtools-core`) `Store` is initialized before the Backend (and
`Agent` as a part of it):

bac33d1f82/packages/react-devtools-extensions/src/main/index.js (L111-L113)

Any messages that we send from `Store`'s constructor are ignored,
because there is nothing on the other end yet. With these changes,
`Agent` will send `backendInitialized` message to `Store`, after which
`getBackendVersion` and other events will be sent.

Note that `isBackendStorageAPISupported` and `isSynchronousXHRSupported`
are still sent from `Agent`'s constructor, because we don't explicitly
ask for it from `Store`, but these are used.

This the pre-requisite for fetching settings and unsupported renderers
reliably from the Frontend.
2024-09-11 15:10:13 +01:00
Sebastian Markbåge
bac33d1f82 [Flight] Unwrap lazy before reading the value (#30938)
This is important if the lazy is at the root of the chunk. I don't have
a unit test for it but @gnoff has a repro.

It also shouldn't unwrap the last value since that's the one we're
referencing.

This was already done correctly by @unstubbable in waitForReference so
this just aligns with that.
2024-09-10 19:42:19 -04:00
Sebastian Markbåge
a5a7f1063e [Fiber] Don't call performance.now() twice in a row (#30936) 2024-09-10 16:46:35 -04:00
Andrew Clark
66cf2cfc8a Prerender during same pass if blocked anyway (#30879)
If something suspends in the shell — i.e. we won't replace the suspended
content with a fallback — we might as well prerender the siblings during
the current render pass, instead of spawning a separate prerender pass.

This is implemented by setting the "is prerendering" flag to true
whenever we suspend in the shell. But only if we haven't already skipped
over some siblings, because if so, then we need to schedule a separate
prerender pass regardless.
2024-09-10 13:23:32 -04:00
Joe Savona
d724ba96ff [compiler] Validate type configs for hooks/non-hooks
Alternative to #30868. The goal is to ensure that the types coming out of moduleTypeProvider are valid wrt to hook typing. If something is named like a hook, then it must be typed as a hook (or don't type it).

ghstack-source-id: 3e8b5a0a7010d0c484bbb417fb258e76bf4e32bc
Pull Request resolved: https://github.com/facebook/react/pull/30888
2024-09-10 09:25:28 -07:00
Ruslan Lesiutin
3dfd5d9efb refactor[RendererInterface]: expose onErrorOrWarning and getComponentStack (#30931)
Make `onErrorOrWarning` and `getComponentStack` part of
`rendererInterface`. By doing this, they will be available from the
global hook `rendererInterfaces` Map. This makes them available to be
used by Hook, which soon will be the only one who is doing console
patching.

This is also a pre-requisite for removing `registerRenderer`:

d160aa0fbb/packages/react-devtools-shared/src/backend/console.js (L113-L121)
2024-09-10 15:59:40 +01:00
Sebastian Markbåge
63cefa2957 [DevTools] Implement getComponentStack and onErrorOrWarning for replayed Flight logs (#30930)
This adds owner stacks to replayed Server Component logs in environments
that don't support native console.createTask.

<img width="521" alt="Screenshot 2024-09-09 at 8 55 21 PM"
src="https://github.com/user-attachments/assets/261cfaee-ea65-4044-abf0-c41abf358fea">

It also tracks the logs in the global componentInfoToComponentLogsMap
which lets us associate those logs with Server Components when they
later commit into the fiber tree.

<img width="1280" alt="Screenshot 2024-09-09 at 9 31 16 PM"
src="https://github.com/user-attachments/assets/436312a6-f9f4-4add-8129-0fb9b9eb18ee">

I tried to create unit tests for this since it's now wired up
end-to-end. Unfortunately, the complicated testing set up for Flight
requires a complex set of resetting modules which are incompatible with
the complicated test setup in getVersionedRenderImplementation for
DevTools tests.
2024-09-10 10:37:29 -04:00
Sebastian Markbåge
d160aa0fbb [DevTools] Use Unicode Atom Symbol instead of Atom Emoji (#30832)
This reverts #19603.

Before:
<img width="724" alt="Screenshot 2024-08-28 at 12 07 29 AM"
src="https://github.com/user-attachments/assets/0613088f-c013-4f1c-92c3-fbdae8c1f109">

After:
<img width="771" alt="Screenshot 2024-08-28 at 12 08 13 AM"
src="https://github.com/user-attachments/assets/eef21bee-d11f-4f0a-9147-053a163f720f">

Consensus seems to be that while the purple on is a bit clearer and
easier to read. The purple is not on brand so it doesn't look like
React. It looks ugly. It's distracting (too eye catching). Taking away
attention from other tabs in an unfair way.

It also gets worse with more tabs added. We plan on both adding another
tab and panes inside other tabs (elements/sources) soon. Each needs to
be marked somehow as part of React but spelling it out is too long.
Putting inside a second tab means two clicks and takes away real-estate
from our extension and doesn't solve the problem with extension panes in
other tabs. We also plan on adding multiple different tracks to the
Performance tab which also needs a name other than just React and
spelling out React as a prefix is too long. The Emoji is too
distracting. So it seems best to uniformly apply the symbol - albeit it
might just look like a dot to many.

Dark mode looks close to on brand:

<img width="1089" alt="Screenshot 2024-08-28 at 12 32 50 AM"
src="https://github.com/user-attachments/assets/7175a540-4241-4c26-9e4d-4d367873af57">
2024-09-10 00:09:42 -04:00
Sebastian Markbåge
6066b8e8e6 [Flight] Reset currentOwnerInDEV (#30929)
Missed this bit.
2024-09-09 22:07:35 -04:00
Sebastian Markbåge
49825c0ffc [Flight] Add react-stack-bottom-frame to console replaying (#30926)
Any time we're creating a stack trace we should have a
react-stack-bottom-frame so we know what to filter out.

This is the same thing we already do for createFakeJSXCallStackInDEV but
we should do that when replaying logs too.
2024-09-09 19:52:05 -04:00
Sebastian Markbåge
0dbacf2041 [DevTools] Improve Layering Between Console and Renderer (#30925)
The console instrumentation should not know about things like Fibers.
Only the renderer bindings should know about that stuff. We can improve
the layering by just moving all that stuff behind a `getComponentStack`
helper that gets injected by the renderer.

This sets us up for the Flight renderer #30906 to have its own
implementation of this function.
2024-09-09 15:33:30 -04:00
Sebastian Markbåge
fa3cf509a9 [DevTools] Add Map for Server Component Logs (#30905)
Stacked on #30899.

This adds another map to store Server Components logs. When they're
replayed with an owner we can associate them with a DevToolsInstance.
The replaying should happen before they can mount in Fiber so they'll
always have all logs when they mount. There can be more than one
Instance associated with any particular ReactComponentInfo. It can also
be unmounted and restored later.

One thing that's interesting about these is that when a Server Component
tree refreshes a new set of ReactComponentInfo will update through the
tree and the VirtualInstances will update with new instances. This means
that the old errors/warnings are no longer associated with the
VirtualInstance. I.e. it's not continually appended like updates do for
Fiber backed instances. On the client we dedupe errors/warnings for the
life time of the page. On the server that doesn't work well because it
would mean that when you refresh the page, you miss out on warnings so
we dedupe them per request instead. If we just appended on refresh it
would keep adding them.

If ever add a deduping mechanism that spans longer than a request, we
might need to do more of a merge when these updates.

Nothing actually adds logs to this map yet. That will need an
integration with Flight in a follow up.
2024-09-09 15:12:28 -04:00
Sebastian Markbåge
f4b3a1fea2 [DevTools] Delete fiberToFiberInstanceMap (#30900)
Stacked on #30899.

After the rest of the stack this is now unused so we can save time and
memory avoiding to maintain it. 🎉
2024-09-09 15:12:13 -04:00
Sebastian Markbåge
2283d7204c [Flight] Inject Client Into DevTools (#30910)
Stacked on #30906.

Injects the Flight Client into the DevTools hook if it `supportsFlight`.
This only injects in DEV. We could inject it in prod too but so far the
only feature this exposes is only available in DEV anyway. I also only
call `injectIntoDevTools` in the browser builds since we don't really
support DevTools on the server anyway.

The main purpose of this for now is so that DevTools can track the
Server Component owner of replayed logs. This lets us add owner stacks
where `console.createTask` is not natively supported (like Firefox). It
also lets us associate the log with the Server Component in the
Component tree #30905.
2024-09-09 15:11:57 -04:00
Sebastian Markbåge
3cac8cd5a9 [DevTools] Add Flight Renderer (#30906)
This represents a virtual renderer that connects to the Flight Client.
It's virtual in the sense that the actual rendering has already happened
on the server. The Flight Client parses the result. Most of the result
then end up in objects that render into another renderer and that's how
we see most Server Components in DevTools. As part of the client's tree.

However, some things are side-effects that don't really connect to any
particular client renderer. For example preloads() and logs. For those
we need to treat the Flight Client as if it was its own renderer just
like a Fiber renderer or even legacy renderer. We really could support
Fizz and Flight Server as DevTools targets too for example to connect it
to the backend but there's just not much demand for that.

This will initially only be used to track the owners of replayed console
logs but could be expanded to more. For example to send controls to
start profiling on the server. It could also be expanded to build an RSC
payload inspector that is automatically connected.
2024-09-09 15:11:34 -04:00
Sebastian Markbåge
e07235b980 [DevTools] Refactor Error / Warning Count Tracking (#30899)
We can simplify this tracking by not having a separate pending set of
logs and the logs tracked per instance and instead we just track the
logs per Fiber. This avoids the need to move it back into the pending
set after unmounts in case a Fiber is reparented.

The main motivation for this is to unify with an upcoming tracking of
logs for Server Components. For those it doesn't make sense to move them
into a per instance set and because the same Server Component - and its
logs - may appear more than once. So no particular instance should steal
it.

The second part of this change is that instead of looking up the
instance from fiber, which requires the fiberToFiberInstanceMap, we
instead look up if a component has any new logs when we traverse it in
the commit phase. After all for a component to have had a log it must
have updated. This is a similar technique to #30897. This technique also
works for Server Components without having to maintain a one to many
relationship from ComponentInfo to VirtualInstance. So it unifies them.

Normally this look up would be fast since the `fiberToComponentsLogs`
set would be empty and so doesn't add any significant weight to the
commit phase. If there's a ton of logs on many different components then
it's not great since it would slow down the commit phase but that's not
what we expect to see so in typical usage, this is better.

There is an unfortunate consequence though which is that
`console.warn/error` in passive effects (i.e. `useEffect`) wouldn't be
picked up because currently we traverse the logs in
`handleCommitFiberRoot` which is too early. If we moved that to
`handlePostCommitFiberRoot` this wouldn't be a problem. In the meantime,
I just detect this and do a brute force flush by walking all mounted
instances if there's a `console.warn/error` inside a passive effect.

If we ever add "owners" to event handlers that are triggered outside the
render/commit phases (like `<div onClick={...}>`) and we want to
associate error/warnings in those, we'd need a different technique to
ensure those get flushed in time.
2024-09-09 15:10:04 -04:00
Sam Zhou
e210d08180 [flow] Upgrade Flow to 0.245.2 (#30919)
## Summary

This PR bumps Flow all the way to the latest 0.245.2. 

Most of the suppressions comes from Flow v0.239.0's change to include
undefined in the return of `Array.pop`.

I also enabled `react.custom_jsx_typing=true` and added custom jsx
typing to match the old behavior that `React.createElement` is
effectively any typed. This is necessary since various builtin
components like `React.Fragment` is actually symbol in the React repo
instead of `React.AbstractComponent<...>`. It can be made more accurate
by customizing the `React$CustomJSXFactory` type, but I will leave it to
the React team to decide.

## How did you test this change?

`yarn flow` for all the renderers
2024-09-09 08:41:44 -07:00
Alex Hunt
984ea11d14 [DevTools] Separate RDT Fusebox into single-panel entry points (#30708)
## Summary

Separate function entry points for `react-devtools-fusebox` into
`initializeComponents` and `initializeProfiler`. The motivation behind
this change is to separate these tabs into top-level Chrome DevTools
panels (aligned with web) in React Native.

## How did you test this change?

- Build `react-devtools-fusebox` and load into local
[rn-chrome-devtools-frontend](https://github.com/facebookexperimental/rn-chrome-devtools-frontend)
project with updated call sites.

<img width="1933" alt="image"
src="https://github.com/user-attachments/assets/202d32a1-b8da-4936-b0e1-04875a30f256">

<img width="1949" alt="image"
src="https://github.com/user-attachments/assets/39dbe154-989c-4f76-b103-aa19f07a3b9c">

 Tabs can be separately initialised in individual Chrome DevTools
panels
2024-09-09 08:12:49 -04:00
Mike Vitousek
a8fc4b1ef8 [compiler][playground] Fix displayed naming of outlined functions
ghstack-source-id: 20c8e9eeba1620b061e41119dfb1a0e15c36645e
Pull Request resolved: https://github.com/facebook/react/pull/30907
2024-09-07 17:50:19 -07:00
Sebastian Markbåge
99cba2b041 [DevTools] Build Updater List from the Commit instead of Map (#30897)
Stacked on #30896.

The problem with the `getUpdatersList` function is that it iterates over
Fibers and then looks up each of those Fibers in the
fiberToFiberInstanceMap which we ideally could get rid of.

However, every time an updater comes into play for a commit it must mean
that something below the updater itself updated and so the updater will
also be cloned which means we'll pass it on the way down when traversing
the tree in the commit.

When we do this traversal, we can just look if the Fiber is in the
updater set and if so add it to the updater list as we go.
2024-09-06 21:59:09 -04:00
Sebastian Markbåge
6292398241 [DevTools] Simplify Context Change Tracking in Profiler (#30896)
When Context change tracking was added to support modern Context it
relied on the "memoizedValue" to read the current value. This only works
in React 18+ when it was added to support Lazy Context Propagation.
However, the backend stored the old value the same way it used to work
for legacy Context in a global map. This was unnecessary since we *also*
have the old value on the previous Fiber.

This removes all the costly tracking of previous values for every Fiber
that uses Contexts slowing down profiling. Instead, we just compare the
Contexts from

The downside is that this no longer supports detecting changes due to
legacy Context because it doesn't have a similar "previous" value.
However, legacy Context has long been deprecated and is completely
removed in 19. So I don't think it's worth supporting since you have to
be on an old version *and* actually use legacy Context *and* trying to
profile something that updates it. Which btw, updating legacy contexts
only worked at all from 16 something when we made updates work. So it
was unusual even in the slight gap where you could and before you had
migrated to modern Context introduced in 16.3.
2024-09-06 21:58:20 -04:00
Sebastian Markbåge
baf47462d6 [DevTools] Remove use of .alternate in root and recordProfilingDurations (#30895)
Ideally we shouldn't use the `.alternate` to access previous state
because ideally Fibers shouldn't have alternates.

The only case it's ok to use it is when it is used to identity the
stateful part of a component's identity. In a non-alternate Fiber model
there would instead be another object that represents instance but in
the current model it's modeled by the pair.

It's not ok is to get the previous state of the tree since that would
not live on the stateful part.

We don't generally need this though because we have the previous state
on instance.data before updating it, or passed from above.
2024-09-06 21:46:09 -04:00
Sebastian Markbåge
d76a5651f4 [DevTools] Handle reordered contexts in Profiler (#30887)
While looking at the Context tracking implementation for other reasons I
noticed this bug.

Originally it wasn't allowed to have conditional `useContext(context)`
(although we did because it's technically possible). With `use(context)`
it is officially allowed to be conditional as long as it is within a
Hook/Component and not within a try/catch.

This means that this loop comparing previous and next contexts need to
consider that the Context objects might not line up and so it's possibly
comparing apples to oranges. We already bailed if one was longer than
the other.

If the order of contexts changes later in the component that means
something else must have already changed earlier so the reason for the
rerender isn't the context so we can just return false in that case.
2024-09-06 21:45:52 -04: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: fc0947ce219334117075df6a4e33b39975af2bc4
Pull Request resolved: https://github.com/facebook/react/pull/30889
2024-09-06 14:32:18 -07:00
Mofei Zhang
bd788b4180 [compiler] Add enablePropagateDepsInHIR flag
Adding new feature flag in preparation for #30894

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

ghstack-source-id: 6b42055b5d28da39d99a235bcd86a82eb7c270f4
Pull Request resolved: https://github.com/facebook/react/pull/30892
2024-09-06 13:02:08 -04:00
Mofei 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åge
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åge
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åge
a06cd9e1d1 [DevTools] Refactor Forcing Fallback / Error of Suspense / Error Boundaries (#30870)
First, this basically reverts
1f3892ef8c
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
mofeiZ
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åge
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åge
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åge
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 Clark
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 Clark
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