Make createDescriptor return a descriptor for components

This moves all logic around legacy descriptors to ReactLegacyDescriptor. This
is responsible for the layer that knows that createClass exports a legacy
factory. When passed one of these classes, it unwraps it to be a real class.

If it is passed a non legacy factory, it is assumed to be a non-react component
that needs to be invoked as a plain function.

The semantic change is that a descriptor is now always returned if passed a
legacy factory. Even if that factory is a mock. A mock would previously return
undefined.

For mocks, I treat the factory as the authoritative function. I call it to extract
the instance or fill it with an empty component placeholder.

Additionally, I make the classes take props as the first argument to the
constructor. This is what the new class system will do.

We currently need to set up some internals by calling the internal construct
method. Instead of doing that automatically in the constructor, I now move that
to a second pass so that mocks can get the plain props.

This means that we can assert that a mock has been called once it's mounted
with it's final props. Instead of the descriptor factory being called.

src/browser/ReactDOM.js

@@ -43,8 +43,8 @@ var mapObject = require('mapObject');
  * @private
  */
 function createDOMComponentClass(omitClose, tag) {
-  var Constructor = function(descriptor) {
-    this.construct(descriptor);
+  var Constructor = function(props) {
+    // This constructor and it's argument is currently used by mocks.
   };
   Constructor.prototype = new ReactDOMComponent(tag, omitClose);
   Constructor.prototype.constructor = Constructor;

src/browser/ReactTextComponent.js

@@ -42,8 +42,8 @@ var mixInto = require('mixInto');
  * @extends ReactComponent
  * @internal
  */
-var ReactTextComponent = function(descriptor) {
-  this.construct(descriptor);
+var ReactTextComponent = function(props) {
+  // This constructor and it's argument is currently used by mocks.
 };
 
 mixInto(ReactTextComponent, ReactComponent.Mixin);

src/browser/ui/React.js

@@ -35,6 +35,7 @@ var ReactDOM = require('ReactDOM');
 var ReactDOMComponent = require('ReactDOMComponent');
 var ReactDefaultInjection = require('ReactDefaultInjection');
 var ReactInstanceHandles = require('ReactInstanceHandles');
+var ReactLegacyDescriptor = require('ReactLegacyDescriptor');
 var ReactMount = require('ReactMount');
 var ReactMultiChild = require('ReactMultiChild');
 var ReactPerf = require('ReactPerf');
@@ -46,20 +47,22 @@ var onlyChild = require('onlyChild');
 
 ReactDefaultInjection.inject();
 
-// TODO: Restore the real create descriptor
-// var createDescriptor = ReactDescriptor.createDescriptor;
-var createDescriptor = function(type, props, children) {
-  // Because of issues with mocks, we temporarily execute the factory function
-  var args = Array.prototype.slice.call(arguments, 1);
-  return type.apply(null, args);
-};
+var createDescriptor = ReactDescriptor.createDescriptor;
 var createFactory = ReactDescriptor.createFactory;
 
 if (__DEV__) {
-  // createDescriptor = ReactDescriptorValidator.createDescriptor;
+  createDescriptor = ReactDescriptorValidator.createDescriptor;
   createFactory = ReactDescriptorValidator.createFactory;
 }
 
+// TODO: Drop legacy descriptors once classes no longer export these factories
+createDescriptor = ReactLegacyDescriptor.wrapCreateDescriptor(
+  createDescriptor
+);
+createFactory = ReactLegacyDescriptor.wrapCreateFactory(
+  createFactory
+);
+
 var React = {
   Children: {
     map: ReactChildren.map,

src/core/ReactCompositeComponent.js

@@ -1350,8 +1350,10 @@ var ReactCompositeComponent = {
    * @public
    */
   createClass: function(spec) {
-    var Constructor = function(props, owner) {
-      this.construct(props, owner);
+    var Constructor = function(props) {
+      // This constructor is overridden by mocks. The argument is used
+      // by mocks to assert on what gets mounted. This will later be used
+      // by the stand-alone class implementation.
     };
     Constructor.prototype = new ReactCompositeComponentBase();
     Constructor.prototype.constructor = Constructor;

src/core/ReactDescriptor.js

@@ -131,33 +131,9 @@ if (__DEV__) {
   defineMutationMembrane(ReactDescriptor.prototype);
 }
 
+ReactDescriptor.prototype._isReactDescriptor = true;
+
 ReactDescriptor.createDescriptor = function(type, config, children) {
-  if (type.isReactLegacyFactory) {
-    // This is probably a legacy factory created by ReactCompositeComponent.
-    // We unwrap it to get to the underlying class.
-    // TODO: Drop this check when we drop ReactLegacyDescriptor.
-    type = type.type;
-  }
-
-  if (typeof type === 'function') {
-    var isReactClass =
-      typeof type.prototype.mountComponent === 'function' &&
-      typeof type.prototype.receiveComponent === 'function';
-    if (!isReactClass) {
-      // This is being called with a plain function we should invoke it
-      // immediately as if this was used with legacy JSX.
-      if (__DEV__) {
-        warning(
-          false,
-          'This JSX uses a plain function. Only React components are valid in' +
-          'JSX.'
-        );
-      }
-      var args = Array.prototype.slice.call(arguments, 1);
-      return type.apply(null, args);
-    }
-  }
-
   var propName;
 
   // Reserved names are extracted
@@ -212,12 +188,6 @@ ReactDescriptor.createDescriptor = function(type, config, children) {
 };
 
 ReactDescriptor.createFactory = function(type) {
-  if (type.isReactLegacyFactory) {
-    // This is probably a legacy factory created by ReactCompositeComponent.
-    // We unwrap it to get to the underlying class.
-    // TODO: Drop this check when we drop ReactLegacyDescriptor.
-    type = type.type;
-  }
   var factory = ReactDescriptor.createDescriptor.bind(null, type);
   // Expose the type on the factory and the prototype so that it can be
   // easily accessed on descriptors. E.g. <Foo />.type === Foo.type.
@@ -264,7 +234,17 @@ ReactDescriptor.isValidFactory = function(factory) {
  * @final
  */
 ReactDescriptor.isValidDescriptor = function(object) {
-  return object instanceof ReactDescriptor;
+  // ReactTestUtils is often used outside of beforeEach where as React is
+  // within it. This leads to two different instances of React on the same
+  // page. To identify a descriptor from a different React instance we use
+  // a flag instead of an instanceof check.
+  var isDescriptor = !!(object && object._isReactDescriptor);
+  // if (isDescriptor && !(object instanceof ReactDescriptor)) {
+  // This is an indicator that you're using multiple versions of React at the
+  // same time. This will screw with ownership and stuff. Fix it, please.
+  // TODO: We could possibly warn here.
+  // }
+  return isDescriptor;
 };
 
 module.exports = ReactDescriptor;

src/core/ReactDescriptorValidator.js

@@ -260,12 +260,6 @@ var ReactDescriptorValidator = {
   },
 
   createFactory: function(type) {
-    if (type.isReactLegacyFactory) {
-      // This is probably a legacy factory created by ReactCompositeComponent.
-      // We unwrap it to get to the underlying class.
-      // TODO: Drop this check when we drop ReactLegacyDescriptor.
-      type = type.type;
-    }
     var validatedFactory = ReactDescriptorValidator.createDescriptor.bind(
       null,
       type

src/core/ReactLegacyDescriptor.js

@@ -18,9 +18,56 @@
 
 "use strict";
 
+var ReactCurrentOwner = require('ReactCurrentOwner');
 var ReactDescriptor = require('ReactDescriptor');
 
 var invariant = require('invariant');
+var monitorCodeUse = require('monitorCodeUse');
+var warning = require('warning');
+
+var legacyFactoryLogs = {};
+function warnForLegacyFactoryCall() {
+  if (!ReactLegacyDescriptorFactory._isLegacyCallWarningEnabled) {
+    return;
+  }
+  var owner = ReactCurrentOwner.current;
+  var name = owner && owner.constructor ? owner.constructor.displayName : 'N/A';
+  if (legacyFactoryLogs.hasOwnProperty(name)) {
+    return;
+  }
+  legacyFactoryLogs[name] = true;
+  // TODO: Warn for this.
+  monitorCodeUse('react_legacy_factory_call', { name: name });
+}
+
+function warnForPlainFunctionType(type) {
+  var isReactClass =
+    typeof type.prototype.mountComponent === 'function' &&
+    typeof type.prototype.receiveComponent === 'function';
+  if (isReactClass) {
+    warning(
+      false,
+      'Did not expect to get a React class here. Use `Component` instead ' +
+      'of `Component.type` or `this.constructor`.'
+    );
+  } else {
+    if (!type._reactWarnedForThisType) {
+      try {
+        type._reactWarnedForThisType = true;
+      } catch (x) {
+        // just incase this is a frozen object or some special object
+      }
+      monitorCodeUse('react_non_component_in_jsx', { name: type.name });
+    }
+    // TODO: This pattern is heavily used by ReactMenu and therefore we
+    // cannot yet warn without spamming users too much.
+    // warning(
+    //   false,
+    //   'This JSX uses a plain function. Only React components are ' +
+    //   'valid in React\'s JSX transform.'
+    // );
+  }
+}
 
 /**
  * Transfer static properties from the source to the target. Functions are
@@ -50,22 +97,86 @@ function proxyStaticMethods(target, source) {
   }
 }
 
+// We use an object instead of a boolean because booleans are ignored by our
+// mocking libraries when these factories gets mocked.
+var LEGACY_MARKER = {};
+
 var ReactLegacyDescriptorFactory = {};
 
+ReactLegacyDescriptorFactory.wrapCreateFactory = function(createFactory) {
+  var legacyCreateFactory = function(type) {
+    if (typeof type !== 'function') {
+      // Non-function types cannot be legacy factories
+      return createFactory(type);
+    }
+
+    if (type.isReactLegacyFactory) {
+      // This is probably a legacy factory created by ReactCompositeComponent.
+      // We unwrap it to get to the underlying class.
+      return createFactory(type.type);
+    }
+
+    if (__DEV__) {
+      warnForPlainFunctionType(type);
+    }
+
+    // Unless it's a legacy factory, then this is probably a plain function,
+    // that is expecting to be invoked by JSX. We can just return it as is.
+    return type;
+  };
+  return legacyCreateFactory;
+};
+
+ReactLegacyDescriptorFactory.wrapCreateDescriptor = function(createDescriptor) {
+  var legacyCreateDescriptor = function(type, props, children) {
+    if (typeof type !== 'function') {
+      // Non-function types cannot be legacy factories
+      return createDescriptor.apply(this, arguments);
+    }
+
+    if (type.isReactLegacyFactory) {
+      // This is probably a legacy factory created by ReactCompositeComponent.
+      // We unwrap it to get to the underlying class.
+      if (type._isMockFunction) {
+        // If this is a mock function, people will expect it to be called. We
+        // will actually call the original mock factory function instead. This
+        // future proofs unit testing that assume that these are classes.
+        type.type._mockedReactClassConstructor = type;
+      }
+      var args = Array.prototype.slice.call(arguments, 0);
+      args[0] = type.type;
+      return createDescriptor.apply(this, args);
+    }
+
+    if (__DEV__) {
+      warnForPlainFunctionType(type);
+    }
+
+    // This is being called with a plain function we should invoke it
+    // immediately as if this was used with legacy JSX.
+    return type.apply(null, Array.prototype.slice.call(arguments, 1));
+  };
+  return legacyCreateDescriptor;
+};
+
 ReactLegacyDescriptorFactory.wrapFactory = function(factory) {
   invariant(
     ReactDescriptor.isValidFactory(factory),
     'This is suppose to accept a descriptor factory'
   );
   var legacyDescriptorFactory = function(config, children) {
-    // This factory should not be called when the new JSX transform is in place.
-    // TODO: Warning - Use JSX instead of direct function calls.
+    // This factory should not be called when JSX is used. Use JSX instead.
+    if (__DEV__) {
+      warnForLegacyFactoryCall();
+    }
     return factory.apply(this, arguments);
   };
   proxyStaticMethods(legacyDescriptorFactory, factory.type);
-  legacyDescriptorFactory.isReactLegacyFactory = true;
+  legacyDescriptorFactory.isReactLegacyFactory = LEGACY_MARKER;
   legacyDescriptorFactory.type = factory.type;
   return legacyDescriptorFactory;
 };
 
+ReactLegacyDescriptorFactory._isLegacyCallWarningEnabled = true;
+
 module.exports = ReactLegacyDescriptorFactory;

src/core/__tests__/ReactDescriptor-test.js

@@ -38,7 +38,7 @@ describe('ReactDescriptor', function() {
   });
 
   it('returns a complete descriptor according to spec', function() {
-    var descriptor = React.createFactory(ComponentClass)();
+    var descriptor = React.createFactory(ComponentFactory)();
     expect(descriptor.type).toBe(ComponentClass);
     expect(descriptor.key).toBe(null);
     expect(descriptor.ref).toBe(null);
@@ -54,20 +54,20 @@ describe('ReactDescriptor', function() {
   });
 
   it('returns an immutable descriptor', function() {
-    var descriptor = React.createFactory(ComponentClass)();
+    var descriptor = React.createFactory(ComponentFactory)();
     expect(() => descriptor.type = 'div').toThrow();
   });
 
   it('does not reuse the original config object', function() {
     var config = { foo: 1 };
-    var descriptor = React.createFactory(ComponentClass)(config);
+    var descriptor = React.createFactory(ComponentFactory)(config);
     expect(descriptor.props.foo).toBe(1);
     config.foo = 2;
     expect(descriptor.props.foo).toBe(1);
   });
 
   it('extracts key and ref from the config', function() {
-    var descriptor = React.createFactory(ComponentClass)({
+    var descriptor = React.createFactory(ComponentFactory)({
       key: '12',
       ref: '34',
       foo: '56'
@@ -79,7 +79,7 @@ describe('ReactDescriptor', function() {
   });
 
   it('coerces the key to a string', function() {
-    var descriptor = React.createFactory(ComponentClass)({
+    var descriptor = React.createFactory(ComponentFactory)({
       key: 12,
       foo: '56'
     });
@@ -90,7 +90,7 @@ describe('ReactDescriptor', function() {
   });
 
   it('preserves the context on the descriptor', function() {
-    var Component = React.createFactory(ComponentClass);
+    var Component = React.createFactory(ComponentFactory);
     var descriptor;
 
     var Wrapper = React.createClass({
@@ -112,7 +112,7 @@ describe('ReactDescriptor', function() {
   });
 
   it('preserves the owner on the descriptor', function() {
-    var Component = React.createFactory(ComponentClass);
+    var Component = React.createFactory(ComponentFactory);
     var descriptor;
 
     var Wrapper = React.createClass({
@@ -136,7 +136,7 @@ describe('ReactDescriptor', function() {
   it('merges an additional argument onto the children prop', function() {
     spyOn(console, 'warn');
     var a = 1;
-    var descriptor = React.createFactory(ComponentClass)({
+    var descriptor = React.createFactory(ComponentFactory)({
       children: 'text'
     }, a);
     expect(descriptor.props.children).toBe(a);
@@ -145,7 +145,7 @@ describe('ReactDescriptor', function() {
 
   it('does not override children if no rest args are provided', function() {
     spyOn(console, 'warn');
-    var descriptor = React.createFactory(ComponentClass)({
+    var descriptor = React.createFactory(ComponentFactory)({
       children: 'text'
     });
     expect(descriptor.props.children).toBe('text');
@@ -154,7 +154,7 @@ describe('ReactDescriptor', function() {
 
   it('overrides children if null is provided as an argument', function() {
     spyOn(console, 'warn');
-    var descriptor = React.createFactory(ComponentClass)({
+    var descriptor = React.createFactory(ComponentFactory)({
       children: 'text'
     }, null);
     expect(descriptor.props.children).toBe(null);
@@ -164,14 +164,14 @@ describe('ReactDescriptor', function() {
   it('merges rest arguments onto the children prop in an array', function() {
     spyOn(console, 'warn');
     var a = 1, b = 2, c = 3;
-    var descriptor = React.createFactory(ComponentClass)(null, a, b, c);
+    var descriptor = React.createFactory(ComponentFactory)(null, a, b, c);
     expect(descriptor.props.children).toEqual([1, 2, 3]);
     expect(console.warn.argsForCall.length).toBe(0);
   });
 
   it('warns for keys for arrays of descriptors in rest args', function() {
     spyOn(console, 'warn');
-    var Component = React.createFactory(ComponentClass);
+    var Component = React.createFactory(ComponentFactory);
 
     Component(null, [ Component(), Component() ]);
 
@@ -183,7 +183,7 @@ describe('ReactDescriptor', function() {
 
   it('does not warn when the descriptor is directly in rest args', function() {
     spyOn(console, 'warn');
-    var Component = React.createFactory(ComponentClass);
+    var Component = React.createFactory(ComponentFactory);
 
     Component(null, Component(), Component());
 
@@ -192,7 +192,7 @@ describe('ReactDescriptor', function() {
 
   it('does not warn when the array contains a non-descriptor', function() {
     spyOn(console, 'warn');
-    var Component = React.createFactory(ComponentClass);
+    var Component = React.createFactory(ComponentFactory);
 
     Component(null, [ {}, {} ]);
 
@@ -248,12 +248,39 @@ describe('ReactDescriptor', function() {
       return 21 + x;
     });
     expect(factory(21)).toBe(42);
-    expect(console.warn.argsForCall.length).toBe(1);
-    expect(console.warn.argsForCall[0][0]).toContain(
-      'This JSX uses a plain function.'
-    );
+    // TODO: This warning is temporarily disabled
+    expect(console.warn.argsForCall.length).toBe(0);
+    // expect(console.warn.argsForCall[0][0]).toContain(
+    //   'This JSX uses a plain function.'
+    // );
   });
 
+  it('warns but allow a plain function to be immediately invoked', function() {
+    spyOn(console, 'warn');
+    var result = React.createDescriptor(function (x, y) {
+      return 21 + x + y;
+    }, 11, 10);
+    expect(result).toBe(42);
+    // TODO: This warning is temporarily disabled
+    expect(console.warn.argsForCall.length).toBe(0);
+    // expect(console.warn.argsForCall[0][0]).toContain(
+    //   'This JSX uses a plain function.'
+    // );
+  });
+
+  it('warns but does not fail on undefined results', function() {
+    spyOn(console, 'warn');
+    var fn = function () { };
+    var result = React.createDescriptor(fn, 1, 2, null);
+    expect(result).toBe(undefined);
+    // TODO: This warning is temporarily disabled
+    expect(console.warn.argsForCall.length).toBe(0);
+    // expect(console.warn.argsForCall[0][0]).toContain(
+    //   'This JSX uses a plain function.'
+    // );
+  });
+
+
   it('should expose the underlying class from a legacy factory', function() {
     var Legacy = React.createClass({ render: function() { } });
     var factory = React.createFactory(Legacy);

src/core/instantiateReactComponent.js

@@ -19,45 +19,102 @@
 
 "use strict";
 
-var invariant = require('invariant');
+var warning = require('warning');
+
+var ReactDescriptor = require('ReactDescriptor');
+var ReactLegacyDescriptor = require('ReactLegacyDescriptor');
+var ReactEmptyComponent = require('ReactEmptyComponent');
 
 /**
- * Validate a `componentDescriptor`. This should be exposed publicly in a follow
- * up diff.
+ * Given `type` and `props` invoke the type function and create an instance from
+ * it. For the normal case it just runs the constructor. For mocks we need to
+ * invoke the convenience constructor and resolve that into an instance.
  *
- * @param {object} descriptor
- * @return {boolean} Returns true if this is a valid descriptor of a Component.
+ * @param {function} type
+ * @param {*} props
+ * @return {object} A new instance of `type`.
+ * @protected
  */
-function isValidComponentDescriptor(descriptor) {
-  return !!descriptor && (
-    (
-      typeof descriptor.type === 'function' &&
-      typeof descriptor.type.prototype.mountComponent === 'function' &&
-      typeof descriptor.type.prototype.receiveComponent === 'function'
-    ) || typeof descriptor.type === 'string'
-  );
+function createInstance(type, props) {
+  if (__DEV__) {
+    if (type._mockedReactClassConstructor) {
+      // If this is a mocked class, we treat the legacy factory as if it was the
+      // class constructor for future proofing unit tests. Because this might
+      // be mocked as a legacy factory, we ignore any warnings triggerd by
+      // this temporary hack.
+      ReactLegacyDescriptor._isLegacyCallWarningEnabled = false;
+      var instance;
+      try {
+        instance = new type._mockedReactClassConstructor(props);
+      } finally {
+        ReactLegacyDescriptor._isLegacyCallWarningEnabled = true;
+      }
+
+      // If the mock implementation was a legacy factory, then it returns a
+      // descriptor. We need to turn this into a real component instance.
+      if (ReactDescriptor.isValidDescriptor(instance)) {
+        type = instance.type;
+        props = instance.props;
+        instance = new type(props);
+      }
+
+      var render = instance.render;
+      if (!render) {
+        // For auto-mocked factories, the prototype isn't shimmed and therefore
+        // there is no render function on the instance. We replace the whole
+        // component with an empty component instance instead.
+        var descriptor = ReactEmptyComponent.getEmptyComponent();
+        type = descriptor.type;
+        props = descriptor.props;
+        instance = new type(props);
+      } else if (render._isMockFunction && !render._getMockImplementation()) {
+        // Auto-mocked components may have a prototype with a mocked render
+        // function. For those, we'll need to mock the result of the render
+        // since we consider undefined to be invalid results from render.
+        render.mockImplementation(
+          ReactEmptyComponent.getEmptyComponent
+        );
+      }
+    }
+  }
+  // Normal case for non-mocks
+  return new type(props);
 }
 
 /**
  * Given a `componentDescriptor` create an instance that will actually be
- * mounted. Currently it just extracts an existing clone from composite
- * components but this is an implementation detail which will change.
+ * mounted.
  *
  * @param {object} descriptor
  * @return {object} A new instance of componentDescriptor's constructor.
  * @protected
  */
 function instantiateReactComponent(descriptor) {
-
-  // TODO: Make warning
-  // if (__DEV__) {
-    invariant(
-      isValidComponentDescriptor(descriptor),
-      'Only React Components are valid for mounting.'
+  if (__DEV__) {
+    warning(
+      descriptor && (typeof descriptor.type === 'function' ||
+                     typeof descriptor.type === 'string'),
+      'Only functions or strings can be mounted as React components.'
+      // Not really strings yet, but as soon as I solve the cyclic dep, they
+      // will be allowed here.
     );
-  // }
+  }
 
-  return new descriptor.type(descriptor);
+  var instance = createInstance(descriptor.type, descriptor.props);
+
+  if (__DEV__) {
+    warning(
+      typeof instance.construct === 'function' &&
+      typeof instance.mountComponent === 'function' &&
+      typeof instance.receiveComponent === 'function',
+      'Only React Components can be mounted.'
+    );
+  }
+
+  // This actually sets up the internal instance. This will become decoupled
+  // from the public instance in a future diff.
+  instance.construct(descriptor);
+  return instance;
 }
 
 module.exports = instantiateReactComponent;

src/test/ReactTestUtils.js

@@ -31,7 +31,6 @@ var ReactUpdates = require('ReactUpdates');
 var SyntheticEvent = require('SyntheticEvent');
 
 var mergeInto = require('mergeInto');
-var copyProperties = require('copyProperties');
 
 var topLevelTypes = EventConstants.topLevelTypes;
 
@@ -246,9 +245,11 @@ var ReactTestUtils = {
       }
     });
 
-    copyProperties(module, ConvenienceConstructor);
     module.mockImplementation(ConvenienceConstructor);
 
+    module.type = ConvenienceConstructor.type;
+    module.isReactLegacyFactory = true;
+
     return this;
   },
 

src/utils/traverseAllChildren.js

@@ -18,6 +18,7 @@
 
 "use strict";
 
+var ReactDescriptor = require('ReactDescriptor');
 var ReactInstanceHandles = require('ReactInstanceHandles');
 var ReactTextComponent = require('ReactTextComponent');
 
@@ -126,8 +127,7 @@ var traverseAllChildrenImpl =
         // All of the above are perceived as null.
         callback(traverseContext, null, storageName, indexSoFar);
         subtreeCount = 1;
-      } else if (children.type && children.type.prototype &&
-                 children.type.prototype.mountComponentIntoNode) {
+      } else if (ReactDescriptor.isValidDescriptor(children)) {
         callback(traverseContext, children, storageName, indexSoFar);
         subtreeCount = 1;
       } else {