DevTools scroll-to error or warning feature broken #22143

Activity

1. bvaughn

   added

   Component: Developer Tools

   on Aug 19, 2021
2. iunfixit commented on Aug 20, 2021

   

   iunfixit

   on Aug 20, 2021

   More actions

   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
3. bvaughn commented on Aug 20, 2021

   

   bvaughn

   on Aug 20, 2021

   ContributorAuthor

   More actions

   I wonder if this is related to a Chrome experimental flag? I can repro in both Chrome stable and canary.
4. eps1lon commented on Aug 20, 2021

   

   eps1lon

   on Aug 20, 2021

   Collaborator

   More actions

   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
5. eps1lon commented on Aug 20, 2021

   

   eps1lon

   on Aug 20, 2021

   Last edited by eps1lon

   Collaborator

   More actions

   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".

   ❤️React with ❤️1Reacted by Brian Vaughn
6. iunfixit commented on Aug 20, 2021

   

   iunfixit

   on Aug 20, 2021

   Last edited by iunfixit

   More actions

   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="/api/browser/proxy?url=https%3A%2F%2Freactjs.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;
   
7. bvaughn

   closed this as completedin #22144on Aug 20, 2021