Inject ReactDOM into ReactWithAddons from ReactWithAddons

We used to read ReactDOM as a global inside ReactAddonsDOMDependenciesUMDShim. This didn't work in AMD environments such as RequireJS and SystemJS. Instead, I changed it so that ReactDOM gets injected into ReactWithAddons by ReactDOM itself. This way we don't have to try to require it (which wouldn't work because AMD doesn't handle circular dependencies well). This means you have to load ReactDOM first before using ReactDOM-dependent addons, but this was already the case before. This commit makes all build fixtures pass.
facebook · Jan 5, 2017 · db5b7ab · db5b7ab
1 parent cebff96
commit db5b7ab
Show file tree

Hide file tree

Showing 8 changed files with 39 additions and 86 deletions.
diff --git a/scripts/fiber/tests-passing-except-dev.txt b/scripts/fiber/tests-passing-except-dev.txt
@@ -1,6 +1,3 @@
-src/addons/transitions/__tests__/ReactTransitionGroup-test.js
-* should warn for duplicated keys with component stack info
-
 src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
 * should not warn when server-side rendering `onScroll`
 * should warn about incorrect casing on properties (ssr)

diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt
@@ -105,6 +105,7 @@ src/addons/transitions/__tests__/ReactTransitionGroup-test.js
 * should handle enter/leave/enter/leave correctly
 * should handle enter/leave/enter correctly
 * should handle entering/leaving several elements at once
+* should warn for duplicated keys
 
 src/isomorphic/children/__tests__/ReactChildren-test.js
 * should support identity for simple

diff --git a/src/addons/ReactAddonsDOMDependencies.js b/src/addons/ReactAddonsDOMDependencies.js
@@ -12,16 +12,11 @@
 'use strict';
 
 var ReactDOM = require('ReactDOM');
-var ReactInstanceMap = require('ReactInstanceMap');
 
 exports.getReactDOM = function() {
   return ReactDOM;
 };
 
-exports.getReactInstanceMap = function() {
-  return ReactInstanceMap;
-};
-
 if (__DEV__) {
   var ReactPerf = require('ReactPerf');
   var ReactTestUtils = require('ReactTestUtils');

diff --git a/src/addons/transitions/ReactTransitionGroup.js b/src/addons/transitions/ReactTransitionGroup.js
@@ -12,7 +12,6 @@
 'use strict';
 
 var React = require('React');
-var ReactAddonsDOMDependencies = require('ReactAddonsDOMDependencies');
 var ReactTransitionChildMapping = require('ReactTransitionChildMapping');
 
 var emptyFunction = require('emptyFunction');
@@ -56,17 +55,9 @@ class ReactTransitionGroup extends React.Component {
   }
 
   componentWillReceiveProps(nextProps) {
-    var nextChildMapping;
-    if (__DEV__) {
-      nextChildMapping = ReactTransitionChildMapping.getChildMapping(
-        nextProps.children,
-        ReactAddonsDOMDependencies.getReactInstanceMap().get(this)._debugID
-      );
-    } else {
-      nextChildMapping = ReactTransitionChildMapping.getChildMapping(
-        nextProps.children
-      );
-    }
+    var nextChildMapping = ReactTransitionChildMapping.getChildMapping(
+      nextProps.children
+    );
     var prevChildMapping = this.state.children;
 
     this.setState({
@@ -129,17 +120,9 @@ class ReactTransitionGroup extends React.Component {
 
     delete this.currentlyTransitioningKeys[key];
 
-    var currentChildMapping;
-    if (__DEV__) {
-      currentChildMapping = ReactTransitionChildMapping.getChildMapping(
-        this.props.children,
-        ReactAddonsDOMDependencies.getReactInstanceMap().get(this)._debugID
-      );
-    } else {
-      currentChildMapping = ReactTransitionChildMapping.getChildMapping(
-        this.props.children
-      );
-    }
+    var currentChildMapping = ReactTransitionChildMapping.getChildMapping(
+      this.props.children
+    );
 
     if (!currentChildMapping || !currentChildMapping.hasOwnProperty(key)) {
       // This was removed before it had fully appeared. Remove it.
@@ -169,17 +152,9 @@ class ReactTransitionGroup extends React.Component {
 
     delete this.currentlyTransitioningKeys[key];
 
-    var currentChildMapping;
-    if (__DEV__) {
-      currentChildMapping = ReactTransitionChildMapping.getChildMapping(
-        this.props.children,
-        ReactAddonsDOMDependencies.getReactInstanceMap().get(this)._debugID
-      );
-    } else {
-      currentChildMapping = ReactTransitionChildMapping.getChildMapping(
-        this.props.children
-      );
-    }
+    var currentChildMapping = ReactTransitionChildMapping.getChildMapping(
+      this.props.children
+    );
 
     if (!currentChildMapping || !currentChildMapping.hasOwnProperty(key)) {
       // This was removed before it had fully entered. Remove it.
@@ -210,17 +185,9 @@ class ReactTransitionGroup extends React.Component {
 
     delete this.currentlyTransitioningKeys[key];
 
-    var currentChildMapping;
-    if (__DEV__) {
-      currentChildMapping = ReactTransitionChildMapping.getChildMapping(
-        this.props.children,
-        ReactAddonsDOMDependencies.getReactInstanceMap().get(this)._debugID
-      );
-    } else {
-      currentChildMapping = ReactTransitionChildMapping.getChildMapping(
-        this.props.children
-      );
-    }
+    var currentChildMapping = ReactTransitionChildMapping.getChildMapping(
+      this.props.children
+    );
 
     if (currentChildMapping && currentChildMapping.hasOwnProperty(key)) {
       // This entered again before it fully left. Add it again.

diff --git a/src/addons/transitions/__tests__/ReactTransitionGroup-test.js b/src/addons/transitions/__tests__/ReactTransitionGroup-test.js
@@ -20,10 +20,6 @@ var ReactTransitionGroup;
 describe('ReactTransitionGroup', () => {
   var container;
 
-  function normalizeCodeLocInfo(str) {
-    return str && str.replace(/\(at .+?:\d+\)/g, '(at **)');
-  }
-
   beforeEach(() => {
     React = require('React');
     ReactDOM = require('ReactDOM');
@@ -296,7 +292,7 @@ describe('ReactTransitionGroup', () => {
     ]);
   });
 
-  it('should warn for duplicated keys with component stack info', () => {
+  it('should warn for duplicated keys', () => {
     spyOn(console, 'error');
 
     class Component extends React.Component {
@@ -315,13 +311,11 @@ describe('ReactTransitionGroup', () => {
       'Child keys must be unique; when two children share a key, ' +
       'only the first child will be used.'
     );
-    expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(1)[0])).toBe(
+    expectDev(console.error.calls.argsFor(1)[0]).toBe(
       'Warning: flattenChildren(...): ' +
       'Encountered two children with the same key, `1`. ' +
       'Child keys must be unique; when two children share a key, ' +
-      'only the first child will be used.\n' +
-      '    in ReactTransitionGroup (at **)\n' +
-      '    in Component (at **)'
+      'only the first child will be used.'
     );
   });
 });
diff --git a/src/umd/ReactDOMUMDEntry.js b/src/umd/ReactDOMUMDEntry.js
@@ -11,24 +11,24 @@
 
 'use strict';
 
+var React = require('React');
 var ReactDOM = require('ReactDOM');
 
-var ReactDOMUMDEntry = Object.assign({
-  __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED: {
-    ReactInstanceMap: require('ReactInstanceMap'),
-  },
-}, ReactDOM);
+var ReactDOMUMDEntry = ReactDOM;
 
 if (__DEV__) {
-  Object.assign(
-    ReactDOMUMDEntry.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED,
-    {
-      // ReactPerf and ReactTestUtils currently only work with the DOM renderer
-      // so we expose them from here, but only in DEV mode.
-      ReactPerf: require('ReactPerf'),
-      ReactTestUtils: require('ReactTestUtils'),
-    }
-  );
+  ReactDOMUMDEntry.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED = {
+    // ReactPerf and ReactTestUtils currently only work with the DOM renderer
+    // so we expose them from here, but only in DEV mode.
+    ReactPerf: require('ReactPerf'),
+    ReactTestUtils: require('ReactTestUtils'),
+  };
+}
+
+// Inject ReactDOM into React for the addons UMD build that depends on ReactDOM (TransitionGroup).
+// We can remove this after we deprecate and remove the addons UMD build.
+if (React.addons) {
+  React.__SECRET_INJECTED_REACT_DOM_DO_NOT_USE_OR_YOU_WILL_BE_FIRED = ReactDOMUMDEntry;
 }
 
 module.exports = ReactDOMUMDEntry;
diff --git a/src/umd/ReactWithAddonsUMDEntry.js b/src/umd/ReactWithAddonsUMDEntry.js
@@ -15,6 +15,7 @@ var ReactWithAddons = require('ReactWithAddons');
 
 // `version` will be added here by the React module.
 var ReactWithAddonsUMDEntry = Object.assign({
+  __SECRET_INJECTED_REACT_DOM_DO_NOT_USE_OR_YOU_WILL_BE_FIRED: null, // Will be injected by ReactDOM UMD build.
   __SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED: {
     ReactCurrentOwner: require('ReactCurrentOwner'),
   },

diff --git a/src/umd/shims/ReactAddonsDOMDependenciesUMDShim.js b/src/umd/shims/ReactAddonsDOMDependenciesUMDShim.js
@@ -9,24 +9,22 @@
  * @providesModule ReactAddonsDOMDependenciesUMDShim
  */
 
-/* globals ReactDOM */
-
 'use strict';
 
-exports.getReactDOM = function() {
-  return ReactDOM;
-};
+function getReactDOM() {
+  var ReactWithAddonsUMDEntry = require('ReactWithAddonsUMDEntry');
+  // This is injected by the ReactDOM UMD build:
+  return ReactWithAddonsUMDEntry.__SECRET_INJECTED_REACT_DOM_DO_NOT_USE_OR_YOU_WILL_BE_FIRED;
+}
 
-exports.getReactInstanceMap = function() {
-  return ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.ReactInstanceMap;
-};
+exports.getReactDOM = getReactDOM;
 
 if (__DEV__) {
   exports.getReactPerf = function() {
-    return ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.ReactPerf;
+    return getReactDOM().__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.ReactPerf;
   };
 
   exports.getReactTestUtils = function() {
-    return ReactDOM.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.ReactTestUtils;
+    return getReactDOM().__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED.ReactTestUtils;
   };
 }