Merge branch 'master' of github.com:facebook/react into unacted-effects-warn

Author: threepointone

.circleci/config.yml

@@ -204,10 +204,8 @@ jobs:
       - run:
           name: Run fuzz tests
           command: |
-            FUZZ_TEST_SEED=$RANDOM
-            echo $FUZZ_TEST_SEED
-            yarn test fuzz --maxWorkers=2
-            yarn test-prod fuzz --maxWorkers=2
+            FUZZ_TEST_SEED=$RANDOM yarn test fuzz --maxWorkers=2
+            FUZZ_TEST_SEED=$RANDOM yarn test-prod fuzz --maxWorkers=2
 
   test_build_prod:
     docker: *docker

packages/react-dom/src/__tests__/ReactDOMFiberAsync-test.internal.js

@@ -10,7 +10,6 @@
 'use strict';
 
 const React = require('react');
-const Fragment = React.Fragment;
 let ReactFeatureFlags = require('shared/ReactFeatureFlags');
 
 let ReactDOM;
@@ -602,84 +601,4 @@ describe('ReactDOMFiberAsync', () => {
       expect(root.createBatch).toBe(undefined);
     });
   });
-
-  describe('Disable yielding', () => {
-    beforeEach(() => {
-      jest.resetModules();
-      ReactFeatureFlags = require('shared/ReactFeatureFlags');
-      ReactFeatureFlags.disableYielding = true;
-      ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false;
-      ReactDOM = require('react-dom');
-      Scheduler = require('scheduler');
-    });
-
-    it('wont yield during a render if yielding is disabled', () => {
-      class A extends React.Component {
-        render() {
-          Scheduler.yieldValue('A');
-          return <div>{this.props.children}</div>;
-        }
-      }
-
-      class B extends React.Component {
-        render() {
-          Scheduler.yieldValue('B');
-          return <div>{this.props.children}</div>;
-        }
-      }
-
-      class C extends React.Component {
-        render() {
-          Scheduler.yieldValue('C');
-          return <div>{this.props.children}</div>;
-        }
-      }
-
-      let root = ReactDOM.unstable_createRoot(container);
-
-      root.render(
-        <Fragment>
-          <A />
-          <B />
-          <C />
-        </Fragment>,
-      );
-
-      expect(Scheduler).toHaveYielded([]);
-
-      Scheduler.unstable_flushNumberOfYields(2);
-      // Even though we just flushed two yields, we should have rendered
-      // everything without yielding when the flag is on.
-      expect(Scheduler).toHaveYielded(['A', 'B', 'C']);
-    });
-
-    it('wont suspend during a render if yielding is disabled', () => {
-      let p = new Promise(resolve => {});
-
-      function Suspend() {
-        throw p;
-      }
-
-      let root = ReactDOM.unstable_createRoot(container);
-      root.render(
-        <React.Suspense fallback={'Loading'}>Initial</React.Suspense>,
-      );
-
-      Scheduler.flushAll();
-      expect(container.textContent).toBe('Initial');
-
-      root.render(
-        <React.Suspense fallback={'Loading'}>
-          <Suspend />
-        </React.Suspense>,
-      );
-
-      expect(Scheduler).toHaveYielded([]);
-
-      Scheduler.flushAll();
-
-      // This should have flushed to the DOM even though we haven't ran the timers.
-      expect(container.textContent).toBe('Loading');
-    });
-  });
 });

packages/react-dom/src/__tests__/ReactDOMSuspensePlaceholder-test.js

@@ -83,25 +83,25 @@ describe('ReactDOMSuspensePlaceholder', () => {
           <div ref={divs[1]}>
             <AsyncText ms={500} text="B" />
           </div>
-          <div style={{display: 'block'}} ref={divs[2]}>
+          <div style={{display: 'inline'}} ref={divs[2]}>
             <Text text="C" />
           </div>
         </Suspense>
       );
     }
     ReactDOM.render(<App />, container);
-    expect(divs[0].current.style.display).toEqual('none !important');
-    expect(divs[1].current.style.display).toEqual('none !important');
-    expect(divs[2].current.style.display).toEqual('none !important');
+    expect(window.getComputedStyle(divs[0].current).display).toEqual('none');
+    expect(window.getComputedStyle(divs[1].current).display).toEqual('none');
+    expect(window.getComputedStyle(divs[2].current).display).toEqual('none');
 
     await advanceTimers(500);
 
     Scheduler.flushAll();
 
-    expect(divs[0].current.style.display).toEqual('');
-    expect(divs[1].current.style.display).toEqual('');
+    expect(window.getComputedStyle(divs[0].current).display).toEqual('block');
+    expect(window.getComputedStyle(divs[1].current).display).toEqual('block');
     // This div's display was set with a prop.
-    expect(divs[2].current.style.display).toEqual('block');
+    expect(window.getComputedStyle(divs[2].current).display).toEqual('inline');
   });
 
   it('hides and unhides timed out text nodes', async () => {
@@ -156,14 +156,14 @@ describe('ReactDOMSuspensePlaceholder', () => {
         ReactDOM.render(<App />, container);
       });
       expect(container.innerHTML).toEqual(
-        '<span style="display: none !important;">Sibling</span><span style=' +
-          '"display: none !important;"></span>Loading...',
+        '<span style="display: none;">Sibling</span><span style=' +
+          '"display: none;"></span>Loading...',
       );
 
       act(() => setIsVisible(true));
       expect(container.innerHTML).toEqual(
-        '<span style="display: none !important;">Sibling</span><span style=' +
-          '"display: none !important;"></span>Loading...',
+        '<span style="display: none;">Sibling</span><span style=' +
+          '"display: none;"></span>Loading...',
       );
 
       await advanceTimers(500);

packages/react-dom/src/client/ReactDOMHostConfig.js

@@ -589,7 +589,12 @@ export function hideInstance(instance: Instance): void {
   // TODO: Does this work for all element types? What about MathML? Should we
   // pass host context to this method?
   instance = ((instance: any): HTMLElement);
-  instance.style.display = 'none !important';
+  const style = instance.style;
+  if (typeof style.setProperty === 'function') {
+    style.setProperty('display', 'none', 'important');
+  } else {
+    style.display = 'none';
+  }
 }
 
 export function hideTextInstance(textInstance: TextInstance): void {

packages/react-fresh/src/ReactFreshBabelPlugin.js

@@ -163,36 +163,6 @@ export default function(babel) {
     return false;
   }
 
-  let hookCalls = new WeakMap();
-
-  function recordHookCall(functionNode, hookCallPath, hookName) {
-    if (!hookCalls.has(functionNode)) {
-      hookCalls.set(functionNode, []);
-    }
-    let hookCallsForFn = hookCalls.get(functionNode);
-    let key = '';
-    if (hookCallPath.parent.type === 'VariableDeclarator') {
-      // TODO: if there is no LHS, consider some other heuristic.
-      key = hookCallPath.parentPath.get('id').getSource();
-    }
-
-    // Some built-in Hooks reset on edits to arguments.
-    const args = hookCallPath.get('arguments');
-    if (hookName === 'useState' && args.length > 0) {
-      // useState second argument is initial state.
-      key += '(' + args[0].getSource() + ')';
-    } else if (hookName === 'useReducer' && args.length > 1) {
-      // useReducer second argument is initial state.
-      key += '(' + args[1].getSource() + ')';
-    }
-
-    hookCallsForFn.push({
-      name: hookName,
-      callee: hookCallPath.node.callee,
-      key,
-    });
-  }
-
   function isBuiltinHook(hookName) {
     switch (hookName) {
       case 'useState':
@@ -299,9 +269,64 @@ export default function(babel) {
 
   let seenForRegistration = new WeakSet();
   let seenForSignature = new WeakSet();
-  let seenForHookCalls = new WeakSet();
   let seenForOutro = new WeakSet();
 
+  let hookCalls = new WeakMap();
+  const HookCallsVisitor = {
+    CallExpression(path) {
+      const node = path.node;
+      const callee = node.callee;
+
+      // Note: this visitor MUST NOT mutate the tree in any way.
+      // It runs early in a separate traversal and should be very fast.
+
+      let name = null;
+      switch (callee.type) {
+        case 'Identifier':
+          name = callee.name;
+          break;
+        case 'MemberExpression':
+          name = callee.property.name;
+          break;
+      }
+      if (name === null || !/^use[A-Z]/.test(name)) {
+        return;
+      }
+      const fnScope = path.scope.getFunctionParent();
+      if (fnScope === null) {
+        return;
+      }
+
+      // This is a Hook call. Record it.
+      const fnNode = fnScope.block;
+      if (!hookCalls.has(fnNode)) {
+        hookCalls.set(fnNode, []);
+      }
+      let hookCallsForFn = hookCalls.get(fnNode);
+      let key = '';
+      if (path.parent.type === 'VariableDeclarator') {
+        // TODO: if there is no LHS, consider some other heuristic.
+        key = path.parentPath.get('id').getSource();
+      }
+
+      // Some built-in Hooks reset on edits to arguments.
+      const args = path.get('arguments');
+      if (name === 'useState' && args.length > 0) {
+        // useState second argument is initial state.
+        key += '(' + args[0].getSource() + ')';
+      } else if (name === 'useReducer' && args.length > 1) {
+        // useReducer second argument is initial state.
+        key += '(' + args[1].getSource() + ')';
+      }
+
+      hookCallsForFn.push({
+        callee: path.node.callee,
+        name,
+        key,
+      });
+    },
+  };
+
   return {
     visitor: {
       ExportDefaultDeclaration(path) {
@@ -617,38 +642,14 @@ export default function(babel) {
           },
         );
       },
-      CallExpression(path) {
-        const node = path.node;
-        const callee = node.callee;
-
-        let name = null;
-        switch (callee.type) {
-          case 'Identifier':
-            name = callee.name;
-            break;
-          case 'MemberExpression':
-            name = callee.property.name;
-            break;
-        }
-        if (name === null || !/^use[A-Z]/.test(name)) {
-          return;
-        }
-
-        // Make sure we're not recording the same calls twice.
-        // This can happen if another Babel plugin replaces parents.
-        if (seenForHookCalls.has(node)) {
-          return;
-        }
-        seenForHookCalls.add(node);
-        // Don't mutate the tree above this point.
-
-        const fn = path.scope.getFunctionParent();
-        if (fn === null) {
-          return;
-        }
-        recordHookCall(fn.block, path, name);
-      },
       Program: {
+        enter(path) {
+          // This is a separate early visitor because we need to collect Hook calls
+          // and "const [foo, setFoo] = ..." signatures before the destructuring
+          // transform mangles them. This extra traversal is not ideal for perf,
+          // but it's the best we can do until we stop transpiling destructuring.
+          path.traverse(HookCallsVisitor);
+        },
         exit(path) {
           const registrations = registrationsByProgramPath.get(path);
           if (registrations === undefined) {

packages/react-fresh/src/__tests__/ReactFresh-test.js

@@ -1361,7 +1361,7 @@ describe('ReactFresh', () => {
       const fallbackChild = container.childNodes[1];
       expect(primaryChild.textContent).toBe('Content 1');
       expect(primaryChild.style.color).toBe('green');
-      expect(primaryChild.style.display).toBe('none !important');
+      expect(primaryChild.style.display).toBe('none');
       expect(fallbackChild.textContent).toBe('Fallback 0');
       expect(fallbackChild.style.color).toBe('green');
       expect(fallbackChild.style.display).toBe('');
@@ -1375,7 +1375,7 @@ describe('ReactFresh', () => {
       expect(container.childNodes[1]).toBe(fallbackChild);
       expect(primaryChild.textContent).toBe('Content 1');
       expect(primaryChild.style.color).toBe('green');
-      expect(primaryChild.style.display).toBe('none !important');
+      expect(primaryChild.style.display).toBe('none');
       expect(fallbackChild.textContent).toBe('Fallback 1');
       expect(fallbackChild.style.color).toBe('green');
       expect(fallbackChild.style.display).toBe('');
@@ -1399,7 +1399,7 @@ describe('ReactFresh', () => {
       expect(container.childNodes[1]).toBe(fallbackChild);
       expect(primaryChild.textContent).toBe('Content 1');
       expect(primaryChild.style.color).toBe('red');
-      expect(primaryChild.style.display).toBe('none !important');
+      expect(primaryChild.style.display).toBe('none');
       expect(fallbackChild.textContent).toBe('Fallback 1');
       expect(fallbackChild.style.color).toBe('red');
       expect(fallbackChild.style.display).toBe('');