Merge pull request #103 from gaearon/sad-one-way

Make DOM selection binding one-way

shells/browser/shared/src/main.js

@@ -85,12 +85,6 @@ function createPanelIfReactLoaded() {
           );
         });
 
-        // Remember if we should sync the browser DevTools to the React tab.
-        // We'll only do that if user intentionally chooses a different React component.
-        bridge.addListener('selectElement', () => {
-          hasReactSelectionChanged = true;
-        });
-
         // This flag lets us tip the Store off early that we expect to be profiling.
         // This avoids flashing a temporary "Profiling not supported" message in the Profiler tab,
         // after a user has clicked the "reload and profile" button.
@@ -168,51 +162,39 @@ function createPanelIfReactLoaded() {
         container._hasInitialHTMLBeenCleared = true;
       }
 
-      function maybeSetReactSelectionFromBrowser() {
+      function setReactSelectionFromBrowser() {
         // When the user chooses a different node in the browser Elements tab,
         // copy it over to the hook object so that we can sync the selection.
         chrome.devtools.inspectedWindow.eval(
-          // Don't reset selection if it didn't change in Elements tab.
-          '(window.__REACT_DEVTOOLS_GLOBAL_HOOK__.$0 !== $0)' +
-            '  ? (window.__REACT_DEVTOOLS_GLOBAL_HOOK__.$0 = $0, true)' +
-            '  : false',
-          (didChangeSelection, error) => {
+          '(window.__REACT_DEVTOOLS_GLOBAL_HOOK__.$0 = $0, undefined)',
+          (_, error) => {
             if (error) {
               console.error(error);
-            } else if (didChangeSelection) {
+            } else {
               bridge.send('syncSelectionFromNativeElementsPanel');
             }
           }
         );
       }
 
-      let hasReactSelectionChanged = false;
-
-      function maybeSetBrowserSelectionFromReact() {
-        // Don't change the browser element selection when navigating away
-        // from the Components tab if the user didn't change the React selection.
-        if (!hasReactSelectionChanged) {
+      // When the user selects another item in the native Elements tab,
+      // select the corresponding React component.
+      let isListeningToNativeSelectionChange = false;
+      function ensureListeningToNativeSelectionChange() {
+        if (isListeningToNativeSelectionChange) {
           return;
         }
-        hasReactSelectionChanged = false;
-        chrome.devtools.inspectedWindow.eval(
-          '(window.__REACT_DEVTOOLS_GLOBAL_HOOK__.$0 != null)' +
-            '  ? (inspect(window.__REACT_DEVTOOLS_GLOBAL_HOOK__.$0), true)' +
-            '  : false',
-          (_, error) => {
-            if (error) {
-              console.error(error);
-            }
-          }
-        );
+        isListeningToNativeSelectionChange = true;
+        setReactSelectionFromBrowser();
+        chrome.devtools.panels.elements.onSelectionChanged.addListener(() => {
+          setReactSelectionFromBrowser();
+        });
       }
 
       let currentPanel = null;
 
       chrome.devtools.panels.create('⚛ Components', '', 'panel.html', panel => {
         panel.onShown.addListener(panel => {
-          maybeSetReactSelectionFromBrowser();
-
           if (currentPanel === panel) {
             return;
           }
@@ -225,9 +207,14 @@ function createPanelIfReactLoaded() {
             render('components');
             panel.injectStyles(cloneStyleTags);
           }
+
+          // Don't start listening to native selection change
+          // until *after* the panel is visible. Otherwise, we'll
+          // set the selected element too early and won't scroll
+          // to it the first time we open Components panel.
+          ensureListeningToNativeSelectionChange();
         });
         panel.onHidden.addListener(() => {
-          maybeSetBrowserSelectionFromReact();
           // TODO: Stop highlighting and stuff.
         });
       });

src/backend/agent.js

@@ -308,22 +308,13 @@ export default class Agent extends EventEmitter {
     if (renderer == null) {
       console.warn(`Invalid renderer id "${rendererID}" for element "${id}"`);
     } else {
-      // When a different React component is selected, we want to store
-      // the active DOM node ($0) on the global hook so that content script
-      // can update the native elements panel to match it.
-      const node = ((renderer.findNativeByFiberID(id): any): HTMLElement);
-      if (node !== null) {
-        // However, we don't want to do it if the current $0 node already
-        // belongs to this component. In this case we were probably inspecting
-        // a part of its host subtree, and changing $0 would be disuptive.
-        const prev$0 = window.__REACT_DEVTOOLS_GLOBAL_HOOK__.$0;
-        const prev$0ID = this.getIDForNode(prev$0);
-        if (prev$0ID !== id) {
-          window.__REACT_DEVTOOLS_GLOBAL_HOOK__.$0 = node;
-        }
-      }
       renderer.selectElement(id);
       this._bridge.send('selectElement');
+      // TODO: If there was a way to change the selected DOM element
+      // in native Elements tab without forcing a switch to it, we'd do it here.
+      // For now, it doesn't seem like there is a way to do that:
+      // https://github.com/bvaughn/react-devtools-experimental/issues/102
+      // (Setting $0 doesn't work, and calling inspect() switches the tab.)
     }
   };