Refactor error/warning count tracking to avoid pre-allocating an ID for Fibers that aren't yet mounted. Instead, we store a temporary reference to the Fiber itself and later check to see if it successfully mounted before merging pending error/warning counts.
This avoids a problematic edge case where a force-remounted Fiber (from Fast Refresh) caused us to untrack a Fiber that was still mounted, resulting in a DevTools error if that Fiber was inspected in the Components tab.
DevTools now 'untrack' Fibers (cleans up the ID-to-Fiber mapping) after a slight delay in order to support a Fast Refresh edge case:
1. Component type is updated and Fast Refresh schedules an update+remount.
2. flushPendingErrorsAndWarningsAfterDelay() runs, sees the old Fiber is no longer mounted (it's been disconnected by Fast Refresh), and calls untrackFiberID() to clear it from the Map.
3. React flushes pending passive effects before it runs the next render, which logs an error or warning, which causes a new ID to be generated for this Fiber.
4. DevTools now tries to unmount the old Component with the new ID.
The underlying problem here is the premature clearing of the Fiber ID, but DevTools has no way to detect that a given Fiber has been scheduled for Fast Refresh. (The '_debugNeedsRemount' flag won't necessarily be set.)
The best we can do is to delay untracking by a small amount, and give React time to process the Fast Refresh delay.
Works around the corrupted Store state by detecting a broken Fast Refresh remount and forcefully dropping the root and re-mounting the entire tree. This prevents Fibers from getting duplicated in the Store (and in the Components tree). The benefit of this approach is that it doesn't rely on an update or change in behavior to Fast Refresh. (This workaround is pretty dirty, but since it's a DEV-only code path, it's probably okay.)
Note that this change doesn't fix all of the reported issues (see #21442 (comment)) but it does fix some of them.
This commit also slightly refactors the way DevTools assigns and manages unique IDs for Fibers in the backend by removing the indirection of a "primary Fiber" and instead mapping both the primary and alternate.
It also removes the previous cache-on-read behavior of getFiberID and splits the method into three separate functions for different use cases:
* getOrGenerateFiberID – Like the previous function, this method returns an ID or generates and caches a new one if the Fiber hasn't been seen before.
* getFiberIDUnsafe – This function returns an ID if one has already been generated or null if not. (It can be used to e.g. log a message about a Fiber without potentially causing it to leak.)
* getFiberIDThrows – This function returns an ID if one has already been generated or it throws. (It can be used to guarantee expected behavior rather than to silently cause a leak.)
Previously, DevTools recursed over both children and siblings during mount. This caused potential stack overflows when there were a lot of children (e.g. a list containing many items).
Given the following example component tree:
A
B C D
E F
G
A method that recurses for every child and sibling leads to a max depth of 6:
A
A -> B
A -> B -> E
A -> B -> C
A -> B -> C -> D
A -> B -> C -> D -> F
A -> B -> C -> D -> F -> G
The stack gets deeper as the tree gets either deeper or wider.
A method that recurses for every child and iterates over siblings leads to a max depth of 4:
A
A -> B
A -> B -> E
A -> C
A -> D
A -> D -> F
A -> D -> F -> G
The stack gets deeper as the tree gets deeper but is resilient to wide trees (e.g. lists containing many items).
Add an explicit Bridge protocol version to the frontend and backend components as well as a check during initialization to ensure that both are compatible. If not, the frontend will display either upgrade or downgrade instructions.
Note that only the `react-devtools-core` (React Native) and `react-devtools-inline` (Code Sandbox) packages implement this check. Browser extensions inject their own backend and so the check is unnecessary. (Arguably the `react-devtools-inline` check is also unlikely to be necessary _but_ has been added as an extra guard for use cases such as Replay.io.)
React has its own component stack generation code that DevTools embeds a fork of, but both of them use a shared helper for disabling console logs. This shared helper is DEV only though, because it was intended for use with React DEV-only warnings and we didn't want to unnecessarily add bytes to production builds.
But DevTools itself always ships as a production build– even when it's used to debug DEV bundles of product apps (with third party DEV-only warnings). That means this helper was always a noop.
The resolveCurrentDispatcher method was changed recently to replace the thrown error with a call to console.error. This newly logged error ended up slipping through and being user visible because of the above issue.
This PR updates DevTools to also fork the console patching logic (to remove the DEV-only guard).
Note that I didn't spot this earlier because my test harness (react-devtools-shell) always runs in DEV mode. 🤡
I recently added UI for the Profiler's commit and post-commit durations to the DevTools, but I made two pretty silly oversights:
1. I used the commit hook (called after mutation+layout effects) to read both the layout and passive effect durations. This is silly because passive effects may not have flushed yet git at this point.
2. I didn't reset the values on the HostRoot node, so they accumulated with each commit.
This commitR addresses both issues:
1. First it adds a new DevTools hook, onPostCommitRoot*, to be called after passive effects get flushed. This gives DevTools the opportunity to read passive effect durations (if the build of React being profiled supports it).
2. Second the work loop resets these durations (on the HostRoot) after calling the post-commit hook so address the accumulation problem.
I've also added a unit test to guard against this regressing in the future.
* Doing this in flushPassiveEffectsImpl seemed simplest, since there are so many places we flush passive effects. Is there any potential problem with this though?
* Remove redundant initial of isArray (#21163)
* Reapply prettier
* Type the isArray function with refinement support
This ensures that an argument gets refined just like it does if isArray is
used directly.
I'm not sure how to express with just a direct reference so I added a
function wrapper and confirmed that this does get inlined properly by
closure compiler.
* A few more
* Rename unit test to internal
This is not testing a bundle.
Co-authored-by: Behnam Mohammadi <itten@live.com>
* Bump version number
* Remove Scheduler indirection
I originally kept the React PriorityLevel and Scheduler PriorityLevel
types separate in case there was a versioning mismatch between the two
modules. However, it looks like we're going to keep the Scheduler module
private in the short to medium term, and longer term the public
interface will match postTask. So, I've removed the extra indirection
(the switch statements that convert between the two types).
* DevTools flushes updated passive warning/error info after delay
Previously this information was not flushed until the next commit, but this provides a worse user experience if the next commit is really delayed. Instead, the backend now flushes only the warning/error counts after a delay. As a safety, if there are already any pending operations in the queue, we bail.
Co-authored-by: eps1lon <silbermann.sebastian@gmail.com>
* Restore inspect-element bridge optimizations
When the new Suspense cache was integrated (so that startTransition could be used) I removed a couple of optimizations between the backend and frontend that reduced bridge traffic when e.g. dehydrated paths were inspected for elements that had not rendered since previously inspected. This commit re-adds those optimizations as well as an additional test with a bug fix that I noticed while reading the backend code.
There are two remaining TODO items as of this commit:
- Make inspected element edits and deletes also use transition API
- Don't over-eagerly refresh the cache in our ping-for-updates handler
I will addres both in subsequent commits.
* Poll for update only refreshes cache when there's an update
* Added inline comment
The memoized state of effect hooks is only invalidated when deps change. Deps are compared between the previous effect and the current effect. This can be problematic if one commit consists of an update that has changed deps followed by an update that has equal deps. That commit will lead to memoizedState containing the changed deps even though we committed with unchanged deps.
The n+1 update will therefore run an effect because we compare the updated deps with the deps with which we never actually committed.
To prevent this we now invalidate memoizedState on every updateEffectImpl call so that memoizedStat.deps always points to the latest deps.
DevTools was built with a fork of an early idea for how Suspense cache might work. This idea is incompatible with newer APIs like `useTransition` which unfortunately prevented me from making certain UX improvements. This PR swaps out the primary usage of this cache (there are a few) in favor of the newer `unstable_getCacheForType` and `unstable_useCacheRefresh` APIs. We can go back and update the others in follow up PRs.
### Messaging changes
I've refactored the way the frontend loads component props/state/etc to hopefully make it better match the Suspense+cache model. Doing this gave up some of the small optimizations I'd added but hopefully the actual performance impact of that is minor and the overall ergonomic improvements of working with the cache API make this worth it.
The backend no longer remembers inspected paths. Instead, the frontend sends them every time and the backend sends a response with those paths. I've also added a new "force" parameter that the frontend can use to tell the backend to send a response even if the component hasn't rendered since the last time it asked. (This is used to get data for newly inspected paths.)
_Initial inspection..._
```
front | | back
| -- "inspect" (id:1, paths:[], force:true) ---------> |
| <------------------------ "inspected" (full-data) -- |
```
_1 second passes with no updates..._
```
| -- "inspect" (id:1, paths:[], force:false) --------> |
| <------------------------ "inspected" (no-change) -- |
```
_User clicks to expand a path, aka hydrate..._
```
| -- "inspect" (id:1, paths:['foo'], force:true) ----> |
| <------------------------ "inspected" (full-data) -- |
```
_1 second passes during which there is an update..._
```
| -- "inspect" (id:1, paths:['foo'], force:false) ---> |
| <----------------- "inspectedElement" (full-data) -- |
```
### Clear errors/warnings transition
Previously this meant there would be a delay after clicking the "clear" button. The UX after this change is much improved.
### Hydrating paths transition
I also added a transition to hydration (expanding "dehyrated" paths).
### Better error boundaries
I also added a lower-level error boundary in case the new suspense operation ever failed. It provides a better "retry" mechanism (select a new element) so DevTools doesn't become entirely useful. Here I'm intentionally causing an error every time I select an element.
### Improved snapshot tests
I also migrated several of the existing snapshot tests to use inline snapshots and added a new serializer for dehydrated props. Inline snapshots are easier to verify and maintain and the new serializer means dehydrated props will be formatted in a way that makes sense rather than being empty (in external snapshots) or super verbose (default inline snapshot format).
* Fixed invalid DevTools work tags
Work tags changed recently (PR #13902) but we didn't bump React versions. This meant that DevTools has valid work tags only for master (and FB www sync) but invalid work tags for the latest open source releases. To fix this, I incremneted React's version in Git (without an actual release) and added a new fork to the work tags detection branch.
This commit also adds tags for the experimental Scope and Fundamental APIs to DevTools so component names will at least display correctly. Technically these new APIs were first introduced to experimental builds ~16.9 but I didn't add a new branch to the work tags fork because I don't they're used commonly. I've just added them to the 17+ branches.
* Removed FundamentalComponent from DevTools tag defs
* Remove Blocks
* Remove Flight Server Runtime
There's no need for this now that the JSResource is part of the bundler
protocol. Might need something for Webpack plugin specifically later.
* Devtools
This is done so that any effects scheduled by the shallow render are thrown away.
Unlike the code this was forked from (in ReactComponentStackFrame) DevTools should override the dispatcher even when DevTools is compiled in production mode, because the app itself may be in development mode and log errors/warnings.
Byron Luk