Use React.autoBind by default.

Per our discussion - this is the general approach we'd like to take for
the public facing API.

    var MyComponent = React.createClass({
      render: function() {
        return <div onClick={this.myCallback} />;
      },
      myCallback: function() {
      }
    });
This commit is contained in:
CommitSyncScript
2013-06-24 16:10:33 -07:00
committed by Paul O’Shannessy
parent 336a0facc1
commit c9ecbaccb3
6 changed files with 217 additions and 195 deletions
+112 -105
View File
@@ -28,6 +28,17 @@ var keyMirror = require('keyMirror');
var merge = require('merge');
var mixInto = require('mixInto');
var invokedBeforeMount = function() {
invariant(
false,
'You have invoked a method that is automatically bound, before the ' +
'instance has been mounted. There is nothing conceptually wrong with ' +
'this, but since this method will be replaced with a new version once ' +
'the component is mounted - you should be aware of the fact that this ' +
'method will soon be replaced.'
);
};
/**
* Policies that describe methods in `ReactCompositeComponentInterface`.
*/
@@ -283,6 +294,47 @@ var RESERVED_SPEC_KEYS = {
}
};
function validateMethodOverride(proto, name) {
var specPolicy = ReactCompositeComponentInterface[name];
// Disallow overriding of base class methods unless explicitly allowed.
if (ReactCompositeComponentMixin.hasOwnProperty(name)) {
invariant(
specPolicy === SpecPolicy.OVERRIDE_BASE,
'ReactCompositeComponentInterface: You are attempting to override ' +
'`%s` from your class specification. Ensure that your method names ' +
'do not overlap with React methods.',
name
);
}
// Disallow defining methods more than once unless explicitly allowed.
if (proto.hasOwnProperty(name)) {
invariant(
specPolicy === SpecPolicy.DEFINE_MANY,
'ReactCompositeComponentInterface: You are attempting to define ' +
'`%s` on your component more than once. This conflict may be due ' +
'to a mixin.',
name
);
}
}
function validateLifeCycleOnReplaceState(instance) {
var compositeLifeCycleState = instance._compositeLifeCycleState;
invariant(
instance.isMounted(),
'replaceState(...): Can only update a mounted component.'
);
invariant(
compositeLifeCycleState !== CompositeLifeCycle.RECEIVING_STATE &&
compositeLifeCycleState !== CompositeLifeCycle.UNMOUNTING,
'replaceState(...): Cannot update while unmounting component or during ' +
'an existing state transition (such as within `render`).'
);
}
/**
* Custom version of `mixInto` which handles policy validation and reserved
* specification keys when building `ReactCompositeComponent` classses.
@@ -290,58 +342,44 @@ var RESERVED_SPEC_KEYS = {
function mixSpecIntoComponent(Constructor, spec) {
var proto = Constructor.prototype;
for (var name in spec) {
if (!spec.hasOwnProperty(name)) {
var property = spec[name];
if (!spec.hasOwnProperty(name) || !property) {
continue;
}
var property = spec[name];
var specPolicy = ReactCompositeComponentInterface[name];
// Disallow overriding of base class methods unless explicitly allowed.
if (ReactCompositeComponentMixin.hasOwnProperty(name)) {
invariant(
specPolicy === SpecPolicy.OVERRIDE_BASE,
'ReactCompositeComponentInterface: You are attempting to override ' +
'`%s` from your class specification. Ensure that your method names ' +
'do not overlap with React methods.',
name
);
}
// Disallow using `React.autoBind` on internal methods.
if (specPolicy != null) {
invariant(
!property || !property.__reactAutoBind,
'ReactCompositeComponentInterface: You are attempting to use ' +
'`React.autoBind` on `%s`, a method that is internal to React.' +
'Internal methods are called with the component as the context.',
name
);
}
// Disallow defining methods more than once unless explicitly allowed.
if (proto.hasOwnProperty(name)) {
invariant(
specPolicy === SpecPolicy.DEFINE_MANY,
'ReactCompositeComponentInterface: You are attempting to define ' +
'`%s` on your component more than once. This conflict may be due ' +
'to a mixin.',
name
);
}
validateMethodOverride(proto, name);
if (RESERVED_SPEC_KEYS.hasOwnProperty(name)) {
RESERVED_SPEC_KEYS[name](Constructor, property);
} else if (property && property.__reactAutoBind) {
if (!proto.__reactAutoBindMap) {
proto.__reactAutoBindMap = {};
}
proto.__reactAutoBindMap[name] = property.__reactAutoBind;
} else if (proto.hasOwnProperty(name)) {
// For methods which are defined more than once, call the existing methods
// before calling the new property.
proto[name] = createChainedFunction(proto[name], property);
} else {
proto[name] = property;
// Setup methods on prototype:
// The following member methods should not be automatically bound:
// 1. Expected ReactCompositeComponent methods (in the "interface").
// 2. Overridden methods (that were mixed in).
var isCompositeComponentMethod = name in ReactCompositeComponentInterface;
var isInherited = name in proto;
var markedDontBind = property.__reactDontBind;
var isFunction = typeof property === 'function';
var shouldAutoBind =
isFunction &&
!isCompositeComponentMethod &&
!isInherited &&
!markedDontBind;
if (shouldAutoBind) {
if (!proto.__reactAutoBindMap) {
proto.__reactAutoBindMap = {};
}
proto.__reactAutoBindMap[name] = property;
proto[name] = invokedBeforeMount;
} else {
if (isInherited) {
// For methods which are defined more than once, call the existing
// methods before calling the new property.
proto[name] = createChainedFunction(proto[name], property);
} else {
proto[name] = property;
}
}
}
}
}
@@ -370,7 +408,26 @@ function createChainedFunction(one, two) {
* `this._compositeLifeCycleState` (which can be null).
*
* This is different from the life cycle state maintained by `ReactComponent` in
* `this._lifeCycleState`.
* `this._lifeCycleState`. The following diagram shows how the states overlap in
* time. There are times when the CompositeLifeCycle is null - at those times it
* is only meaningful to look at ComponentLifeCycle alone.
*
* Top Row: ReactComponent.ComponentLifeCycle
* Low Row: ReactComponent.CompositeLifeCycle
*
* +-------+------------------------------------------------------+--------+
* | UN | MOUNTED | UN |
* |MOUNTED| | MOUNTED|
* +-------+------------------------------------------------------+--------+
* | ^--------+ +------+ +------+ +------+ +--------^ |
* | | | | | | | | | | | |
* | 0--|MOUNTING|-0-|RECEIV|-0-|RECEIV|-0-|RECEIV|-0-| UN |--->0 |
* | | | |PROPS | | PROPS| | STATE| |MOUNTING| |
* | | | | | | | | | | | |
* | | | | | | | | | | | |
* | +--------+ +------+ +------+ +------+ +--------+ |
* | | | |
* +-------+------------------------------------------------------+--------+
*/
var CompositeLifeCycle = keyMirror({
/**
@@ -429,11 +486,7 @@ var ReactCompositeComponentMixin = {
*/
mountComponent: function(rootID, transaction) {
ReactComponent.Mixin.mountComponent.call(this, rootID, transaction);
// Unset `this._lifeCycleState` until after this method is finished.
this._lifeCycleState = ReactComponent.LifeCycle.UNMOUNTED;
this._compositeLifeCycleState = CompositeLifeCycle.MOUNTING;
this._processProps(this.props);
if (this.__reactAutoBindMap) {
@@ -459,15 +512,11 @@ var ReactCompositeComponentMixin = {
// Done with mounting, `setState` will now trigger UI changes.
this._compositeLifeCycleState = null;
this._lifeCycleState = ReactComponent.LifeCycle.MOUNTED;
var html = this._renderedComponent.mountComponent(rootID, transaction);
var markup = this._renderedComponent.mountComponent(rootID, transaction);
if (this.componentDidMount) {
transaction.getReactOnDOMReady().enqueue(this, this.componentDidMount);
}
return html;
return markup;
},
/**
@@ -551,18 +600,7 @@ var ReactCompositeComponentMixin = {
*/
replaceState: function(completeState) {
var compositeLifeCycleState = this._compositeLifeCycleState;
invariant(
this.isMounted() ||
compositeLifeCycleState === CompositeLifeCycle.MOUNTING,
'replaceState(...): Can only update a mounted (or mounting) component.'
);
invariant(
compositeLifeCycleState !== CompositeLifeCycle.RECEIVING_STATE &&
compositeLifeCycleState !== CompositeLifeCycle.UNMOUNTING,
'replaceState(...): Cannot update while unmounting component or during ' +
'an existing state transition (such as within `render`).'
);
validateLifeCycleOnReplaceState.call(null, this);
this._pendingState = completeState;
// Do not trigger a state transition if we are in the middle of mounting or
@@ -583,7 +621,6 @@ var ReactCompositeComponentMixin = {
transaction
);
ReactComponent.ReactReconcileTransaction.release(transaction);
this._compositeLifeCycleState = null;
}
},
@@ -777,25 +814,12 @@ var ReactCompositeComponentMixin = {
*/
_bindAutoBindMethod: function(method) {
var component = this;
var hasWarned = false;
function autoBound(a, b, c, d, e, tooMany) {
invariant(
typeof tooMany === 'undefined',
'React.autoBind(...): Methods can only take a maximum of 5 arguments.'
);
if (component._lifeCycleState === ReactComponent.LifeCycle.MOUNTED) {
return method.call(component, a, b, c, d, e);
} else if (!hasWarned) {
hasWarned = true;
if (__DEV__) {
console.warn(
'React.autoBind(...): Attempted to invoke an auto-bound method ' +
'on an unmounted instance of `%s`. You either have a memory leak ' +
'or an event handler that is being run after unmounting.',
component.constructor.displayName || 'ReactCompositeComponent'
);
}
}
return method.call(component, a, b, c, d, e);
}
return autoBound;
}
@@ -850,33 +874,16 @@ var ReactCompositeComponent = {
},
/**
* Marks the provided method to be automatically bound to the component.
* This means the method's context will always be the component.
*
* React.createClass({
* handleClick: React.autoBind(function() {
* this.setState({jumping: true});
* }),
* render: function() {
* return <a onClick={this.handleClick}>Jump</a>;
* }
* });
* TODO: Delete this when all callers have been updated to rely on this
* behavior being the default.
*
* Backwards compatible stub for what is now the default behavior.
* @param {function} method Method to be bound.
* @public
*/
autoBind: function(method) {
function unbound() {
invariant(
false,
'React.autoBind(...): Attempted to invoke an auto-bound method that ' +
'was not correctly defined on the class specification.'
);
}
unbound.__reactAutoBind = method;
return unbound;
return method;
}
};
module.exports = ReactCompositeComponent;
+48
View File
@@ -0,0 +1,48 @@
/**
* Copyright 2013 Facebook, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
* @providesModule ReactDoNotBindDeprecated
*/
var ReactDoNotBindDeprecated = {
/**
* Marks the method for not being automatically bound on component mounting. A
* couple of reasons you might want to use this:
*
* - Automatically supporting the previous behavior in components that were
* built with previous versions of React.
* - Tuning performance, by avoiding binding on initial render for methods
* that are always invoked while being preceded by `this.`. Such binds are
* unnecessary.
*
* React.createClass({
* handleClick: ReactDoNotBindDeprecated.doNotBind(function() {
* alert(this.setState); // undefined!
* }),
* render: function() {
* return <a onClick={this.handleClick}>Jump</a>;
* }
* });
*
* @param {function} method Method to avoid automatically binding.
* @public
*/
doNotBind: function(method) {
method.__reactDontBind = true; // Mutating
return method;
}
};
module.exports = ReactDoNotBindDeprecated;
+1 -1
View File
@@ -30,7 +30,7 @@ var nodeCache = {};
* other objects so just return '' if we're given something other than a
* DOM node (such as window).
*
* @param {DOMElement|DOMWindow|DOMDocument} node DOM node.
* @param {?DOMElement|DOMWindow|DOMDocument|DOMTextNode} node DOM node.
* @returns {string} ID of the supplied `domNode`.
*/
function getID(node) {
+13 -6
View File
@@ -21,9 +21,11 @@
var mocks = require('mocks');
var React = require('React');
var ReactDoNotBindDeprecated = require('ReactDoNotBindDeprecated');
var ReactTestUtils = require('ReactTestUtils');
var reactComponentExpect = require('reactComponentExpect');
// TODO: Test render and all stock methods.
describe('React.autoBind', function() {
it('Holds reference to instance', function() {
@@ -31,16 +33,20 @@ describe('React.autoBind', function() {
var mouseDidEnter = mocks.getMockFunction();
var mouseDidLeave = mocks.getMockFunction();
var mouseDidClick = mocks.getMockFunction();
var didBadIdea = mocks.getMockFunction();
var TestBindComponent = React.createClass({
onMouseEnter: mouseDidEnter,
onMouseLeave: mouseDidLeave,
getInitialState: function() {
return {something: 'hi'};
},
onMouseEnter: ReactDoNotBindDeprecated.doNotBind(mouseDidEnter),
onMouseLeave: ReactDoNotBindDeprecated.doNotBind(mouseDidLeave),
onClick: React.autoBind(mouseDidClick),
// autoBind needs to be on the top-level spec.
// auto binding only occurs on top level functions in class defs.
badIdeas: {
badBind: React.autoBind(didBadIdea)
badBind: function() {
this.state.something;
}
},
render: function() {
@@ -48,7 +54,8 @@ describe('React.autoBind', function() {
<div
onMouseEnter={this.onMouseEnter.bind(this)}
onMouseLeave={this.onMouseLeave}
onClick={this.onClick} />
onClick={this.onClick}
/>
);
}
});
@@ -286,6 +286,8 @@ describe('ReactComponentLifeCycle', function() {
if (isInitialRender) {
this._testJournal.stateInInitialRender = clone(this.state);
this._testJournal.lifeCycleInInitialRender = this._lifeCycleState;
this._testJournal.compositeLifeCycleInInitialRender =
this._compositeLifeCycleState;
} else {
this._testJournal.stateInLaterRender = clone(this.state);
this._testJournal.lifeCycleInLaterRender = this._lifeCycleState;
@@ -319,7 +321,7 @@ describe('ReactComponentLifeCycle', function() {
GET_INIT_STATE_RETURN_VAL
);
expect(instance._testJournal.lifeCycleAtStartOfGetInitialState)
.toBe(ComponentLifeCycle.UNMOUNTED);
.toBe(ComponentLifeCycle.MOUNTED);
expect(instance._testJournal.compositeLifeCycleAtStartOfGetInitialState)
.toBe(CompositeComponentLifeCycle.MOUNTING);
@@ -328,7 +330,7 @@ describe('ReactComponentLifeCycle', function() {
instance._testJournal.returnedFromGetInitialState
);
expect(instance._testJournal.lifeCycleAtStartOfWillMount)
.toBe(ComponentLifeCycle.UNMOUNTED);
.toBe(ComponentLifeCycle.MOUNTED);
expect(instance._testJournal.compositeLifeCycleAtStartOfWillMount)
.toBe(CompositeComponentLifeCycle.MOUNTING);
@@ -343,7 +345,10 @@ describe('ReactComponentLifeCycle', function() {
expect(instance._testJournal.stateInInitialRender)
.toEqual(INIT_RENDER_STATE);
expect(instance._testJournal.lifeCycleInInitialRender).toBe(
ComponentLifeCycle.UNMOUNTED
ComponentLifeCycle.MOUNTED
);
expect(instance._testJournal.compositeLifeCycleInInitialRender).toBe(
CompositeComponentLifeCycle.MOUNTING
);
expect(instance._lifeCycleState).toBe(ComponentLifeCycle.MOUNTED);
@@ -429,70 +434,5 @@ describe('ReactComponentLifeCycle', function() {
expect(instance.state.stateField).toBe('goodbye');
});
it('should call nested lifecycle methods in the right order', function() {
var log;
var logger = function(msg) {
return function() {
// return true for shouldComponentUpdate
log.push(msg);
return true;
};
};
var Outer = React.createClass({
render: function() {
return <div><Inner x={this.props.x} /></div>;
},
componentWillMount: logger('outer componentWillMount'),
componentDidMount: logger('outer componentDidMount'),
componentWillReceiveProps: logger('outer componentWillReceiveProps'),
shouldComponentUpdate: logger('outer shouldComponentUpdate'),
componentWillUpdate: logger('outer componentWillUpdate'),
componentDidUpdate: logger('outer componentDidUpdate'),
componentWillUnmount: logger('outer componentWillUnmount')
});
var Inner = React.createClass({
render: function() {
return <span>{this.props.x}</span>;
},
componentWillMount: logger('inner componentWillMount'),
componentDidMount: logger('inner componentDidMount'),
componentWillReceiveProps: logger('inner componentWillReceiveProps'),
shouldComponentUpdate: logger('inner shouldComponentUpdate'),
componentWillUpdate: logger('inner componentWillUpdate'),
componentDidUpdate: logger('inner componentDidUpdate'),
componentWillUnmount: logger('inner componentWillUnmount')
});
var instance;
log = [];
instance = ReactTestUtils.renderIntoDocument(<Outer x={17} />);
expect(log).toEqual([
'outer componentWillMount',
'inner componentWillMount',
'inner componentDidMount',
'outer componentDidMount'
]);
log = [];
instance.setProps({x: 42});
expect(log).toEqual([
'outer componentWillReceiveProps',
'outer shouldComponentUpdate',
'outer componentWillUpdate',
'inner componentWillReceiveProps',
'inner shouldComponentUpdate',
'inner componentWillUpdate',
'inner componentDidUpdate',
'outer componentDidUpdate'
]);
log = [];
instance.unmountComponent();
expect(log).toEqual([
'outer componentWillUnmount',
'inner componentWillUnmount'
]);
});
});
@@ -27,6 +27,7 @@ var ReactCurrentOwner;
var ReactProps;
var ReactTestUtils;
var ReactID;
var ReactDoNotBindDeprecated;
var cx;
var reactComponentExpect;
@@ -38,6 +39,7 @@ describe('ReactCompositeComponent', function() {
reactComponentExpect = require('reactComponentExpect');
React = require('React');
ReactCurrentOwner = require('ReactCurrentOwner');
ReactDoNotBindDeprecated = require('ReactDoNotBindDeprecated');
ReactProps = require('ReactProps');
ReactTestUtils = require('ReactTestUtils');
ReactID = require('ReactID');
@@ -52,7 +54,7 @@ describe('ReactCompositeComponent', function() {
},
render: function() {
var toggleActivatedState = this._toggleActivatedState.bind(this);
var toggleActivatedState = this._toggleActivatedState;
return !this.state.activated ?
<a ref="x" onClick={toggleActivatedState} /> :
<b ref="x" onClick={toggleActivatedState} />;
@@ -64,9 +66,9 @@ describe('ReactCompositeComponent', function() {
return {activated: false};
},
_toggleActivatedState: React.autoBind(function() {
_toggleActivatedState:function() {
this.setState({activated: !this.state.activated});
}),
},
render: function() {
return !this.state.activated ?
@@ -167,15 +169,18 @@ describe('ReactCompositeComponent', function() {
});
it('should auto bind methods and values correctly', function() {
var RETURN_VALUE_AFTER_MOUNT = 'returnValue';
var ComponentClass = React.createClass({
getInitialState: function() {
return {
valueToReturn: RETURN_VALUE_AFTER_MOUNT
};
return {valueToReturn: 'hi'};
},
methodBoundOnMount: React.autoBind(function() {
return this.state.valueToReturn;
methodToBeExplicitlyBound: function() {
return this;
},
methodAutoBound: function() {
return this;
},
methodExplicitlyNotBound: ReactDoNotBindDeprecated.doNotBind(function() {
return this;
}),
render: function() {
return <div> </div>;
@@ -184,18 +189,33 @@ describe('ReactCompositeComponent', function() {
var instance = <ComponentClass />;
// Autobound methods will throw before mounting.
// TODO: We should actually allow component instance methods to be invoked
// before mounting for read-only operations. We would then update this test.
expect(function() {
instance.methodBoundOnMount();
instance.methodToBeExplicitlyBound.bind(instance)();
}).toThrow();
expect(function() {
instance.methodAutoBound();
}).toThrow();
expect(function() {
instance.methodExplicitlyNotBound();
}).not.toThrow();
// Next, prove that once mounted, the scope is bound correctly to the actual
// component.
ReactTestUtils.renderIntoDocument(instance);
var retValAfterMount = instance.methodBoundOnMount();
expect(retValAfterMount).toBe(RETURN_VALUE_AFTER_MOUNT);
var retValAfterMountWithCrazyScope =
instance.methodBoundOnMount.call({thisIsACrazyScope:null});
expect(retValAfterMountWithCrazyScope).toBe(RETURN_VALUE_AFTER_MOUNT);
var explicitlyBound = instance.methodToBeExplicitlyBound.bind(instance);
var autoBound = instance.methodAutoBound;
var explicitlyNotBound = instance.methodExplicitlyNotBound;
expect(explicitlyBound.call(null)).toBe(instance);
expect(autoBound.call(null)).toBe(instance);
expect(explicitlyNotBound.call(null)).toBe(null);
expect(explicitlyBound.call(instance)).toBe(instance);
expect(autoBound.call(instance)).toBe(instance);
expect(explicitlyNotBound.call(instance)).toBe(instance);
});
it('should normalize props with default values', function() {