A slight behavior change here too is that I now mark the start of the
commit phase before the BeforeMutationEffect phase. This affects
`<Profiler>` too.
The named sequences are as follows:
Render -> Suspended or Throttled -> Commit -> Waiting for Paint ->
Remaining Effects
The Suspended phase is only logged if we delay the Commit due to CSS /
images.
The Throttled phase is only logged if we delay the commit due to the
Suspense throttling timer.
<img width="1246" alt="Screenshot 2024-09-20 at 9 14 23 PM"
src="https://github.com/user-attachments/assets/8d01f444-bb85-472b-9b42-6157d92c81b4">
I don't yet log render phases that don't complete. I think I also need
to special case renders that or don't commit after being suspended.
<!--
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
Profiling fails sometimes because `onProfilingStatus` is called
repeatedly on some occasions, e.g. multiple calls to
`getProfilingStatus`.
Subsequent calls should be a no-op if the profiling status hasn't
changed.
Reported via #30661#28838.
> [!TIP]
> Hide whitespace changes on this PR
<img width="328" alt="screenshot showing the UI controls for hiding
whitespace changes on GitHub"
src="https://github.com/user-attachments/assets/036385cf-2610-4e69-a717-17c05d7ef047">
## 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.
-->
Tested as part of Fusebox implementation of reload-to-profile.
https://github.com/facebook/react/pull/31021?#discussion_r1770589753
This tracks the current window.event.timeStamp the first time we
setState or call startTransition. For either the blocking track or
transition track. We can use this to show how long we were blocked by
other events or overhead from when the user interacted until we got
called into React.
Then we track the time we start awaiting a Promise returned from
startTransition. We can use this track how long we waited on an Action
to complete before setState was called.
Then finally we track when setState was called so we can track how long
we were blocked by other word before we could actually start rendering.
For a Transition this might be blocked by Blocking React render work.
We only log these once a subsequent render actually happened. If no
render was actually scheduled, then we don't log these. E.g. if an
isomorphic Action doesn't call startTransition there's no render so we
don't log it.
We only log the first event/update/transition even if multiple are
batched into it later. If multiple Actions are entangled they're all
treated as one until an update happens. If no update happens and all
entangled actions finish, we clear the transition so that the next time
a new sequence starts we can log it.
We also clamp these (start the track later) if they were scheduled
within a render/commit. Since we share a single track we don't want to
create overlapping tracks.
The purpose of this is not to show every event/action that happens but
to show a prelude to how long we were blocked before a render started.
So you can follow the first event to commit.
<img width="674" alt="Screenshot 2024-09-20 at 1 59 58 AM"
src="https://github.com/user-attachments/assets/151ba9e8-6b3c-4fa1-9f8d-e3602745eeb7">
I still need to add the rendering/suspended phases to the timeline which
why this screenshot has a gap.
<img width="993" alt="Screenshot 2024-09-20 at 12 50 27 AM"
src="https://github.com/user-attachments/assets/155b6675-b78a-4a22-a32b-212c15051074">
In this case it's a Form Action which started a render into the form
which then suspended on the action. The action then caused a refresh,
which interrupts with its own update that's blocked before rendering.
Suspended roots like this is interesting because we could in theory
start working on a different root in the meantime which makes this
timeline less linear.
When aborting we currently don't produce a componentStack when aborting
the shell. This is likely just an oversight and this change updates this
behavior to be consistent with what we do when there is a boundary
## Overview
Adds a lint rule to prevent optional chaining to catch issues like
https://github.com/facebook/react/pull/30982 until we support optional
chaining without a bundle impact.
Stacked on https://github.com/facebook/react/pull/30986.
Previously, we would call `installHook` at a top level of the JavaScript
module. Because of this, having `require` statement for
`react-devtools-core` package was enough to initialize the React
DevTools global hook on the `window`.
Now, the Hook can actually receive an argument - initial user settings
for console patching. We expose this as a function `initialize`, which
can be used by third parties (including React Native) to provide the
persisted settings.
The README was also updated to reflect the changes.
Rewrite `containerInfo?.ownerDocument?.defaultView ?? window` to instead
use a ternary.
This changes the compilation output (see [bundle changes from
#30951](https://github.com/facebook/react/commit/d65fb06955e9f32e6a40d1c7177d77893dff95b9)).
```js
// compilation of containerInfo?.ownerDocument?.defaultView ?? window
var $jscomp$optchain$tmpm1756096108$1, $jscomp$nullish$tmp0;
containerInfo =
null !=
($jscomp$nullish$tmp0 =
null == containerInfo
? void 0
: null ==
($jscomp$optchain$tmpm1756096108$1 = containerInfo.ownerDocument)
? void 0
: $jscomp$optchain$tmpm1756096108$1.defaultView)
? $jscomp$nullish$tmp0
: window;
// compilation of ternary expression
containerInfo =
null != containerInfo &&
null != containerInfo.ownerDocument &&
null != containerInfo.ownerDocument.defaultView
? containerInfo.ownerDocument.defaultView
: window;
```
This also reduces the number of no-op bundle syncs for Meta. Note that
Closure compiler's `jscomp$optchain$tmp<HASH>` identifiers change when
we rebuild (likely due to version number changes). See
[workflow](https://github.com/facebook/react/actions/runs/10891164281/job/30221518374)
for a PR that was synced despite making no changes to the runtime.
Stacked on https://github.com/facebook/react/pull/30566 and whats under
it. See [this
commit](https://github.com/facebook/react/pull/30596/commits/374fd737e4b0b7028afb765838db7c0e22def865).
It is mostly copying code from one place to another and updating tests.
With these changes, for every console method that we patch, there is
going to be a single applied patch:
- For `error`, `warn`, and `trace` we are patching when hook is
installed. This guarantees that component stacks are going to be
appended even if browser DevTools are not opened. We pay some price for
it, though: if user has browser DevTools closed and if at this point
some warning or error is emitted (logged), the next time user opens
browser DevTools, they are going to see `hook.js` as the source frame.
Unfortunately, ignore listing from source maps is not applied
retroactively, and I don't know if its a bug or just a design
limitations. Once browser DevTools are opened, source maps will be
loaded and ignore listing will be applied for all emitted logs in the
future.
- For `log`, `info`, `group`, `groupCollapsed` we are only patching when
React notifies React DevTools about running in StrictMode. We unpatch
the methods right after it.
Right now we are patching console 2 times: when hook is installed
(before page is loaded) and when backend is connected. Because of this,
even if user had `appendComponentStack` setting enabled, all emitted
error and warning logs are not going to have component stacks appended.
They also won't have component stacks appended retroactively when user
opens browser DevTools (this is when frontend is initialized and
connects to backend).
This behavior adds potential race conditions with LogBox in React
Native, and also unpredictable to the user, because in order to get
component stacks logged you have to open browser DevTools, but by the
time you do it, error or warning log was already emitted.
To solve this, we are going to only patch console in the hook object,
because it is guaranteed to load even before React. Settings are going
to be synchronized with the hook via Bridge, and React DevTools Backend
Host (React Native or browser extension shell) will be responsible for
persisting these settings across the session, this is going to be
implemented in a separate PR.
Stacked on #30981. Same as #30967 but for effects.
This logs a tree of components using `performance.measure()`.
In addition to the previous render phase this logs one tree for each
commit phase:
- Mutation Phase
- Layout Effect
- Passive Unmounts
- Passive Mounts
I currently skip the Before Mutation phase since the snapshots are so
unusual it's not worth creating trees for those.
The mechanism is that I reuse the timings we track for
`enableProfilerCommitHooks`. I track first and last effect timestamp
within each component subtree. Then on the way up do we log the entry.
This means that we don't include overhead to find our way down to a
component and that we don't need to add any additional overhead by
reading timestamps.
To ensure that the entries get ordered correctly we need to ensure that
the start time of each parent is slightly before the inner one.
This code is weird. It reads back the transition that it just set from
the shared internals. It's almost like it expects it to be a getter or
something.
This avoids that and makes it consistent with what ReactFiberHooks
already does.
Stacked on #30979.
The problem with the previous approach is that it recursively walked the
tree up to propagate the resulting time from recording a layout effect.
Instead, we keep a running count of the effect duration on the module
scope. Then we reset it when entering a nested Profiler and then we add
its elapsed count when we exit the Profiler.
This also fixes a bug where we weren't previously including unmount
times for some detached trees since they couldn't bubble up to find the
profiler.
Stacked on #30960 and #30966. Behind the enableComponentPerformanceTrack
flag.
This is the first step of performance logging. This logs the start and
end time of a component render in the passive effect phase. We use the
data we're already tracking on components when the Profiler component or
DevTools is active in the Profiling or Dev builds. By backdating this
after committing we avoid adding more overhead in the hot path. By only
logging things that actually committed, we avoid the costly unwinding of
an interrupted render which was hard to maintain in earlier versions.
We already have the start time but we don't have the end time. That's
because `actualStartTime + actualDuration` isn't enough since
`actualDuration` counts the actual CPU time excluding yields and
suspending in the render.
Instead, we infer the end time to be the start time of the next sibling
or the complete time of the whole root if there are no more siblings. We
need to pass this down the passive effect tree. This will mean that any
overhead and yields are attributed to this component's span. In a follow
up, we'll need to start logging these yields to make it clear that this
is not part of the component's self-time.
In follow ups, I'll do the same for commit phases. We'll also need to
log more information about the phases in the top track. We'll also need
to filter out more components from the trees that we don't need to
highlight like the internal Offscreen components. It also needs polish
on colors etc.
Currently, I place the components into separate tracks depending on
which lane currently committed. That way you can see what was blocking
Transitions or Suspense etc. One problem that I've hit with the new
performance.measure extensions is that these tracks show up in the order
they're used which is not the order of priority that we use. Even when
you add fake markers they have to actually be within the performance run
since otherwise the calls are noops so it's not enough to do that once.
However, I think this visualization is actually not good because these
trees end up so large that you can't see any other lanes once you expand
one. Therefore, I think in a follow up I'll actually instead switch to a
model where Components is a single track regardless of lane since we
don't currently have overlap anyway. Then the description about what is
actually rendering can be separate lanes.
<img width="1512" alt="Screenshot 2024-09-15 at 10 55 55 PM"
src="https://github.com/user-attachments/assets/5ca3fa74-97ce-40c7-97f7-80c1dd7d6470">
<img width="1512" alt="Screenshot 2024-09-15 at 10 56 27 PM"
src="https://github.com/user-attachments/assets/557ad65b-4190-465f-843c-0bc6cbb9326d">
We used to queue a separate third passive phase to invoke onPostCommit
but this is unnecessary. We can just treat it as a plain passive effect.
This means it is interleaved with other passive effects but we only need
to know the duration of the things below us which is already done at
this point.
I also extracted the user space call to onPostCommit into
ReactCommitEffects. Same as onCommit. It's now covered by
runWithFiberInDEV and catches.
This flag will be used to gate a new timeline profiler that's integrate
with the Performance Tab and the new performance.measure extensions in
Chrome.
It replaces the existing DevTools feature so this disables
enableSchedulingProfiler when it is enabled since they can interplay in
weird ways potentially.
This means that experimental React now disable scheduling profiler and
enables this new approach.
In a past update we made render and prerender have different work
scheduling behavior because these methods are meant to be used in
differeent environments with different performance tradeoffs in mind.
For instance to prioritize streaming we want to allow as much IO to
complete before triggering a round of work because we want to flush as
few intermediate UI states. With Prerendering there will never be any
intermediate UI states so we can more aggressively render tasks as they
complete.
One thing we've found is that even during render we should ideally kick
off work immediately. This update normalizes the intitial work for
render and prerender to start in a microtask. Choosing microtask over
sync is somewhat arbitrary but there really isn't a reason to make them
different between render/prerender so for now we'll unify them and keep
it as a microtask for now.
This change also updates pinging behavior. If the request is still in
the initial task that spawned it then pings will schedule on the
microtask queue. This allows immediately available async APIs to resolve
right away. The concern with doing this for normal pings is that it
might crowd out IO events but since this is the initial task there would
be IO to already be scheduled.
Nit: I don't trust flags in hot code. While it can take somewhat longer
to compile two functions and JIT them. After that they don't need to
check branches. Also makes it clearer the purpose.
When the environment name changes for a chunk we issue a new debug chunk
which updates the environment name. This chunk was not beign included in
the pendingChunks count so the count was off when flushing
In #26624, the ability to mark a client reference module as `async` in
the React client manifest was removed because it was not utilized by
Webpack, neither in `ReactFlightWebpackPlugin` nor in Next.js. However,
some bundlers and frameworks are sophisticated enough to properly handle
and identify async ESM modules (e.g., client component modules with
top-level `await`), most notably Turbopack in Next.js. Therefore, we
need to consider the `async` flag in the client manifest when resolving
the client reference metadata on the server. The SSR manifest cannot
override this flag, meaning that if a module is async, it must remain
async in all client environments.
x-ref: https://github.com/vercel/next.js/pull/70022
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>
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.
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.
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.
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.
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.
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.
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):
https://github.com/facebook/react/blob/bac33d1f82d9094b45d6f662dd7fa895abab8bce/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.
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.
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.
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.
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">
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.