React DevTools v4 integration

Summary:
This Diff is being posted for discussion purposes. It will not be ready to land until React DevTools v4 has been published to NPM.

Update React Native to be compatible with the [new version 4 React DevTools extension](https://github.com/bvaughn/react-devtools-experimental).

**Note that this is a breaking change**, as the version 3 and version 4 backends are **not compatible**. Once this update ships (in React Native) users will be required to update their version of the [`react-devtools` NPM package](https://www.npmjs.com/package/react-devtools). The same will be true for IDEs like Nuclide as well as other developer tools like Flipper and [React Native Debugger](https://github.com/jhen0409/react-native-debugger).

Related changes also included in this diff are:
* Pass an explicit whitelist of style props for the React Native style editor (to improve developer experience when adding new styles).
* Update `YellowBox` console patching to coordinate with DevTools own console patching.
  * Also improved formatting slightly by not calling `stringifySafe` for strings (since this adds visible quotation marks).

Regarding the console patching- component stacks will be appended by default when there's no DevTools frontend open. The frontend will provide an option to turn this behavior off though:

{F168852162}

React DevTools will detect if the new version is used with an older version of React Native, and offer inline upgrade instructions:

{F169306863}

**Note that the change to the `RCTEnableTurboModule` will not be included in this Diff**. I've just turned those off temporarily so I can use v8+Chrome for debugging.

Reviewed By: rickhanlonii

Differential Revision: D15973709

fbshipit-source-id: bb9d83fc829af4693e7a10a622acc95a411a48e4

Author: Brian Vaughn

Libraries/Core/setUpDeveloperTools.js

@@ -39,13 +39,18 @@ if (__DEV__) {
           ? devServer.url.replace(/https?:\/\//, '').split(':')[0]
           : 'localhost';
 
+        const viewConfig = require('../Components/View/ReactNativeViewViewConfig.js');
+
         reactDevTools.connectToDevTools({
           isAppActive,
           host,
           // Read the optional global variable for backward compatibility.
           // It was added in https://github.com/facebook/react-native/commit/bf2b435322e89d0aeee8792b1c6e04656c2719a0.
           port: window.__REACT_DEVTOOLS_PORT__,
           resolveRNStyle: require('../StyleSheet/flattenStyle'),
+          nativeStyleEditorValidAttributes: Object.keys(
+            viewConfig.validAttributes.style,
+          ),
         });
       }
 

Libraries/Inspector/Inspector.js

@@ -30,13 +30,16 @@ export type ReactRenderer = {
 const hook = window.__REACT_DEVTOOLS_GLOBAL_HOOK__;
 const renderers = findRenderers();
 
-// required for devtools to be able to edit react native styles
+// Required for React DevTools to view/edit React Native styles in Flipper.
+// Flipper doesn't inject these values when initializing DevTools.
 hook.resolveRNStyle = require('../StyleSheet/flattenStyle');
+const viewConfig = require('../Components/View/ReactNativeViewViewConfig.js');
+hook.nativeStyleEditorValidAttributes = Object.keys(
+  viewConfig.validAttributes.style,
+);
 
 function findRenderers(): $ReadOnlyArray<ReactRenderer> {
-  const allRenderers = Object.keys(hook._renderers).map(
-    key => hook._renderers[key],
-  );
+  const allRenderers = Array.from(hook.renderers.values());
   invariant(
     allRenderers.length >= 1,
     'Expected to find at least one React Native renderer on DevTools hook.',
@@ -78,6 +81,7 @@ class Inspector extends React.Component<
     networking: boolean,
   },
 > {
+  _hideTimeoutID: TimeoutID | null = null;
   _subs: ?Array<() => void>;
 
   constructor(props: Object) {
@@ -97,64 +101,78 @@ class Inspector extends React.Component<
   }
 
   componentDidMount() {
-    hook.on('react-devtools', this.attachToDevtools);
+    hook.on('react-devtools', this._attachToDevtools);
     // if devtools is already started
     if (hook.reactDevtoolsAgent) {
-      this.attachToDevtools(hook.reactDevtoolsAgent);
+      this._attachToDevtools(hook.reactDevtoolsAgent);
     }
   }
 
   componentWillUnmount() {
     if (this._subs) {
       this._subs.map(fn => fn());
     }
-    hook.off('react-devtools', this.attachToDevtools);
+    hook.off('react-devtools', this._attachToDevtools);
   }
 
   UNSAFE_componentWillReceiveProps(newProps: Object) {
     this.setState({inspectedViewTag: newProps.inspectedViewTag});
   }
 
-  attachToDevtools: (agent: any) => void = (agent: Object) => {
-    let _hideWait = null;
-    const hlSub = agent.sub('highlight', ({node, name, props}) => {
-      clearTimeout(_hideWait);
+  _attachToDevtools = (agent: Object) => {
+    agent.addListener('hideNativeHighlight', this._onAgentHideNativeHighlight);
+    agent.addListener('showNativeHighlight', this._onAgentShowNativeHighlight);
+    agent.addListener('shutdown', this._onAgentShutdown);
 
-      if (typeof node !== 'number') {
-        // Fiber
-        node = ReactNative.findNodeHandle(node);
-      }
+    this.setState({
+      devtoolsAgent: agent,
+    });
+  };
 
-      UIManager.measure(node, (x, y, width, height, left, top) => {
-        this.setState({
-          hierarchy: [],
-          inspected: {
-            frame: {left, top, width, height},
-            style: props ? props.style : {},
-          },
-        });
+  _onAgentHideNativeHighlight = () => {
+    if (this.state.inspected === null) {
+      return;
+    }
+    // we wait to actually hide in order to avoid flicker
+    this._hideTimeoutID = setTimeout(() => {
+      this.setState({
+        inspected: null,
+      });
+    }, 100);
+  };
+
+  _onAgentShowNativeHighlight = node => {
+    clearTimeout(this._hideTimeoutID);
+
+    if (typeof node !== 'number') {
+      node = ReactNative.findNodeHandle(node);
+    }
+
+    UIManager.measure(node, (x, y, width, height, left, top) => {
+      this.setState({
+        hierarchy: [],
+        inspected: {
+          frame: {left, top, width, height},
+        },
       });
     });
-    const hideSub = agent.sub('hideHighlight', () => {
-      if (this.state.inspected === null) {
-        return;
-      }
-      // we wait to actually hide in order to avoid flicker
-      _hideWait = setTimeout(() => {
-        this.setState({
-          inspected: null,
-        });
-      }, 100);
-    });
-    this._subs = [hlSub, hideSub];
+  };
+
+  _onAgentShutdown = () => {
+    const agent = this.state.devtoolsAgent;
+    if (agent != null) {
+      agent.removeListener(
+        'hideNativeHighlight',
+        this._onAgentHideNativeHighlight,
+      );
+      agent.removeListener(
+        'showNativeHighlight',
+        this._onAgentShowNativeHighlight,
+      );
+      agent.removeListener('shutdown', this._onAgentShutdown);
 
-    agent.on('shutdown', () => {
       this.setState({devtoolsAgent: null});
-      this._subs = null;
-    });
-    this.setState({
-      devtoolsAgent: agent,
-    });
+    }
   };
 
   setSelection(i: number) {
@@ -187,11 +205,7 @@ class Inspector extends React.Component<
     if (this.state.devtoolsAgent) {
       // Skip host leafs
       const offsetFromLeaf = hierarchy.length - 1 - selection;
-      this.state.devtoolsAgent.selectFromDOMNode(
-        touchedViewTag,
-        true,
-        offsetFromLeaf,
-      );
+      this.state.devtoolsAgent.selectNode(touchedViewTag);
     }
 
     this.setState({

Libraries/YellowBox/Data/YellowBoxCategory.js

@@ -60,9 +60,13 @@ const YellowBoxCategory = {
 
         if (substitutionIndex < substitutionCount) {
           if (substitutionIndex < substitutions.length) {
-            const substitution = stringifySafe(
-              substitutions[substitutionIndex],
-            );
+            // Don't stringify a string type.
+            // It adds quotation mark wrappers around the string,
+            // which causes the yellow box to look odd.
+            const substitution =
+              typeof substitutions[substitutionIndex] === 'string'
+                ? substitutions[substitutionIndex]
+                : stringifySafe(substitutions[substitutionIndex]);
             substitutionOffsets.push({
               length: substitution.length,
               offset: contentString.length,
@@ -88,7 +92,12 @@ const YellowBoxCategory = {
       contentParts.push(contentString);
     }
 
-    const remainingArgs = remaining.map(stringifySafe);
+    const remainingArgs = remaining.map(arg => {
+      // Don't stringify a string type.
+      // It adds quotation mark wrappers around the string,
+      // which causes the yellow box to look odd.
+      return typeof arg === 'string' ? arg : stringifySafe(arg);
+    });
     categoryParts.push(...remainingArgs);
     contentParts.push(...remainingArgs);
 

Libraries/YellowBox/Data/YellowBoxWarning.js

@@ -34,8 +34,28 @@ class YellowBoxWarning {
     message: Message,
     stack: Stack,
   |} {
+    let mutableArgs: Array<mixed> = [...args];
+
+    // This detects a very narrow case of a simple warning string,
+    // with a component stack appended by React DevTools.
+    // In this case, we convert the component stack to a substituion,
+    // because YellowBox formats those pleasantly.
+    // If there are other subtituations or formatting,
+    // we bail to avoid potentially corrupting the data.
+    if (mutableArgs.length === 2) {
+      const first = mutableArgs[0];
+      const last = mutableArgs[1];
+      if (
+        typeof first === 'string' &&
+        typeof last === 'string' &&
+        /^\n {4}in/.exec(last)
+      ) {
+        mutableArgs[0] = first + '%s';
+      }
+    }
+
     return {
-      ...YellowBoxCategory.parse(args),
+      ...YellowBoxCategory.parse(mutableArgs),
       stack: createStack({framesToPop: framesToPop + 1}),
     };
   }

Libraries/YellowBox/Data/__tests__/YellowBoxCategory-test.js

@@ -26,9 +26,9 @@ describe('YellowBoxCategory', () => {
 
   it('parses strings with arguments', () => {
     expect(YellowBoxCategory.parse(['A', 'B', 'C'])).toEqual({
-      category: 'A "B" "C"',
+      category: 'A B C',
       message: {
-        content: 'A "B" "C"',
+        content: 'A B C',
         substitutions: [],
       },
     });
@@ -38,10 +38,10 @@ describe('YellowBoxCategory', () => {
     expect(YellowBoxCategory.parse(['%s', 'A'])).toEqual({
       category: '\ufeff%s',
       message: {
-        content: '"A"',
+        content: 'A',
         substitutions: [
           {
-            length: 3,
+            length: 1,
             offset: 0,
           },
         ],
@@ -53,15 +53,15 @@ describe('YellowBoxCategory', () => {
     expect(YellowBoxCategory.parse(['%s %s', 'A'])).toEqual({
       category: '\ufeff%s %s',
       message: {
-        content: '"A" %s',
+        content: 'A %s',
         substitutions: [
           {
-            length: 3,
+            length: 1,
             offset: 0,
           },
           {
             length: 2,
-            offset: 4,
+            offset: 2,
           },
         ],
       },
@@ -70,12 +70,12 @@ describe('YellowBoxCategory', () => {
 
   it('parses formatted strings with excess arguments', () => {
     expect(YellowBoxCategory.parse(['%s', 'A', 'B'])).toEqual({
-      category: '\ufeff%s "B"',
+      category: '\ufeff%s B',
       message: {
-        content: '"A" "B"',
+        content: 'A B',
         substitutions: [
           {
-            length: 3,
+            length: 1,
             offset: 0,
           },
         ],
@@ -85,12 +85,12 @@ describe('YellowBoxCategory', () => {
 
   it('treats "%s" in arguments as literals', () => {
     expect(YellowBoxCategory.parse(['%s', '%s', 'A'])).toEqual({
-      category: '\ufeff%s "A"',
+      category: '\ufeff%s A',
       message: {
-        content: '"%s" "A"',
+        content: '%s A',
         substitutions: [
           {
-            length: 4,
+            length: 2,
             offset: 0,
           },
         ],
@@ -111,10 +111,10 @@ describe('YellowBoxCategory', () => {
     expect(
       YellowBoxCategory.render(
         {
-          content: '"A"',
+          content: 'A',
           substitutions: [
             {
-              length: 3,
+              length: 1,
               offset: 0,
             },
           ],
@@ -128,19 +128,19 @@ describe('YellowBoxCategory', () => {
     expect(
       YellowBoxCategory.render(
         {
-          content: '"A" "B" "C"',
+          content: 'A B C',
           substitutions: [
             {
-              length: 3,
+              length: 1,
               offset: 0,
             },
             {
-              length: 3,
-              offset: 4,
+              length: 1,
+              offset: 2,
             },
             {
-              length: 3,
-              offset: 8,
+              length: 1,
+              offset: 4,
             },
           ],
         },
@@ -153,10 +153,10 @@ describe('YellowBoxCategory', () => {
     expect(
       YellowBoxCategory.render(
         {
-          content: '!"A"',
+          content: '!A',
           substitutions: [
             {
-              length: 3,
+              length: 1,
               offset: 1,
             },
           ],
@@ -170,10 +170,10 @@ describe('YellowBoxCategory', () => {
     expect(
       YellowBoxCategory.render(
         {
-          content: '"A"!',
+          content: 'A!',
           substitutions: [
             {
-              length: 3,
+              length: 1,
               offset: 0,
             },
           ],