DevTools scroll-to error or warning feature broken

[bvaughn]

Not sure when this happened, but the feature got broken:

scroll-to-broken.mp4

Using the up/down arrows to navigate the list works fine, but jumping to the next/previous component with a warning or error doesn't quite work correctly. (The scroll offset is wrong so the component is often not visible.)

[iunfixit]

I cannot reproduce this unless I'm doing something wrong, tested with errors here

The part where it is selected outside of the view is because of the recording, it never goes behind the console

[bvaughn]

I wonder if this is related to a Chrome experimental flag? I can repro in both Chrome stable and canary.

[eps1lon]

I can reproduce on Version 92.0.4515.159 (Official Build) (64-bit) (with and without our good friend "Throttle non-visible cross-origin iframes" enabled).

Though it does scroll. It just is a bit offset. It also seems to work on upward movement if the current node is already in the frame.

Notice how the scrollbar actually moves:

devtools-scroll-to-warning.mp4

[eps1lon]

When we call store.getElementsWithErrorsAndWarnings() we get a cached list of elements with their indices and ids. The problem is that we only update this list when errors or warning change. But the indices of these elements can change with other operations as well so we need to invalidate the cache more often (or at least update the indices).

If I just naively invalidate the cache on every operation, the issue is no longer reproducible:

diff --git a/packages/react-devtools-shared/src/devtools/store.js b/packages/react-devtools-shared/src/devtools/store.js
index 16bc56ae64..af2ed945c4 100644
--- a/packages/react-devtools-shared/src/devtools/store.js
+++ b/packages/react-devtools-shared/src/devtools/store.js
@@ -1124,7 +1124,7 @@ export default class Store extends EventEmitter<{|
 
     this._revision++;
 
-    if (haveErrorsOrWarningsChanged) {
+    if (true) {
       let errorCount = 0;
       let warningCount = 0;
 

I may have an idea how to update the cache more "smartly".

[iunfixit]

I wonder if this is related to a Chrome experimental flag? I can repro in both Chrome stable and canary.

I only tested with the default instead of enabled, because when it is enabled it freezes everything, I am using Brave 1.28.105 Chromium: 92.0.4515.131, DevTools version 4.16.0-1d2528097. Same behavior on Chromium(without Brave) 92.0.4515.159.
I also notice sometimes the tool doesn't work, unless I change the code and it hot reloads

The code is just modified from the latest CRA

import logo from './logo.svg';
import './App.css';
import React from 'react'

const Err = () => [`a`, `b`, `d`].map(i => <p key={1}>something</p>);
const TenDivs = () => <><div/><div/></>
const Div = () => <div><Err/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><TenDivs/><Err/><Err/></div>;

function App() {
  return (
    <div className="App">
      <header className="App-header"> <Div/>
        <img src={logo} className="App-logo" alt="logo" />
        <p>
          Edit <code>src/App.js</code> and save to reload.
        </p> 
        <a
          className="App-link"
          href="https://reactjs.org"
          target="_blank"
          rel="noopener noreferrer"
        >
          Learn React
        </a> <Div/> <Div/> <Div/> <Div/>
          <Div/>
          <Div/>
          <Div/>
          <Div/>
          <Div/>
          <Div/>
          <Div/>
          <Div/>
          <Div/>
          <Div/>
        <Err/>
      </header> 
    </div>
  );
}

export default App;