diff --git a/src/browser/ReactDOM.js b/src/browser/ReactDOM.js index 306a602254..5c666d7adf 100644 --- a/src/browser/ReactDOM.js +++ b/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; diff --git a/src/browser/ReactTextComponent.js b/src/browser/ReactTextComponent.js index 24ea723729..e04fca6d26 100644 --- a/src/browser/ReactTextComponent.js +++ b/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); diff --git a/src/browser/ui/React.js b/src/browser/ui/React.js index f12ed7dbf5..8948fb2577 100644 --- a/src/browser/ui/React.js +++ b/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, diff --git a/src/core/ReactCompositeComponent.js b/src/core/ReactCompositeComponent.js index 9d3d716b72..ba87210452 100644 --- a/src/core/ReactCompositeComponent.js +++ b/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; diff --git a/src/core/ReactDescriptor.js b/src/core/ReactDescriptor.js index e89daff9fd..1818736460 100644 --- a/src/core/ReactDescriptor.js +++ b/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. .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; diff --git a/src/core/ReactDescriptorValidator.js b/src/core/ReactDescriptorValidator.js index 83dae9b0a0..4123160d17 100644 --- a/src/core/ReactDescriptorValidator.js +++ b/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 diff --git a/src/core/ReactLegacyDescriptor.js b/src/core/ReactLegacyDescriptor.js index e6ba9efa77..06d8dd37c0 100644 --- a/src/core/ReactLegacyDescriptor.js +++ b/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; diff --git a/src/core/__tests__/ReactDescriptor-test.js b/src/core/__tests__/ReactDescriptor-test.js index 64d70070b6..0e767346d7 100644 --- a/src/core/__tests__/ReactDescriptor-test.js +++ b/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); diff --git a/src/core/instantiateReactComponent.js b/src/core/instantiateReactComponent.js index 14efc47057..ed95778b96 100644 --- a/src/core/instantiateReactComponent.js +++ b/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; diff --git a/src/test/ReactTestUtils.js b/src/test/ReactTestUtils.js index d56ad26b31..d8ed3c981f 100644 --- a/src/test/ReactTestUtils.js +++ b/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; }, diff --git a/src/utils/traverseAllChildren.js b/src/utils/traverseAllChildren.js index 3127c74fdd..07e41b359d 100644 --- a/src/utils/traverseAllChildren.js +++ b/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 {