Skip to content

Apply component and mixins specs deterministically #1601

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 16, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/docs/05-reusable-components.md
Original file line number Diff line number Diff line change
Expand Up @@ -187,5 +187,5 @@ React.renderComponent(
);
```

A nice feature of mixins is that if a component is using multiple mixins and several mixins define the same lifecycle method (i.e. several mixins want to do some cleanup when the component is destroyed), all of the lifecycle methods are guaranteed to be called.
A nice feature of mixins is that if a component is using multiple mixins and several mixins define the same lifecycle method (i.e. several mixins want to do some cleanup when the component is destroyed), all of the lifecycle methods are guaranteed to be called. Methods defined on mixins run in the order mixins were listed, followed by a method call on the component.

18 changes: 17 additions & 1 deletion src/core/ReactCompositeComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,16 @@ var ReactUpdates = require('ReactUpdates');
var instantiateReactComponent = require('instantiateReactComponent');
var invariant = require('invariant');
var keyMirror = require('keyMirror');
var keyOf = require('keyOf');
var merge = require('merge');
var mixInto = require('mixInto');
var monitorCodeUse = require('monitorCodeUse');
var mapObject = require('mapObject');
var shouldUpdateReactComponent = require('shouldUpdateReactComponent');
var warning = require('warning');

var MIXINS_KEY = keyOf({mixins: null});

/**
* Policies that describe methods in `ReactCompositeComponentInterface`.
*/
Expand Down Expand Up @@ -453,12 +456,25 @@ function mixSpecIntoComponent(ConvenienceConstructor, spec) {

var Constructor = ConvenienceConstructor.type;
var proto = Constructor.prototype;

// By handling mixins before any other properties, we ensure the same
// chaining order is applied to methods with DEFINE_MANY policy, whether
// mixins are listed before or after these methods in the spec.
if (spec.hasOwnProperty(MIXINS_KEY)) {
RESERVED_SPEC_KEYS.mixins(ConvenienceConstructor, spec.mixins);
}

for (var name in spec) {
var property = spec[name];
if (!spec.hasOwnProperty(name)) {
continue;
}

if (name === MIXINS_KEY) {
// We have already handled mixins in a special case above
continue;
}

var property = spec[name];
validateMethodOverride(proto, name);

if (RESERVED_SPEC_KEYS.hasOwnProperty(name)) {
Expand Down
31 changes: 31 additions & 0 deletions src/core/__tests__/ReactCompositeComponentMixin-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ var reactComponentExpect;

var TestComponent;
var TestComponentWithPropTypes;
var TestComponentWithReverseSpec;
var mixinPropValidator;
var componentPropValidator;

Expand Down Expand Up @@ -58,6 +59,13 @@ describe('ReactCompositeComponent-mixin', function() {
}
};

var MixinBWithReverseSpec = {
componentDidMount: function() {
this.props.listener('MixinBWithReverseSpec didMount');
},
mixins: [MixinA]
};

var MixinC = {
statics: {
staticC: function() {}
Expand Down Expand Up @@ -89,6 +97,16 @@ describe('ReactCompositeComponent-mixin', function() {
}
});

TestComponentWithReverseSpec = React.createClass({
render: function() {
return <div />;
},
componentDidMount: function() {
this.props.listener('Component didMount');
},
mixins: [MixinBWithReverseSpec, MixinC, MixinD]
});

TestComponentWithPropTypes = React.createClass({
mixins: [MixinD],
propTypes: {
Expand Down Expand Up @@ -128,6 +146,19 @@ describe('ReactCompositeComponent-mixin', function() {
]);
});

it('should chain functions regardless of spec property order', function() {
var listener = mocks.getMockFunction();
var instance = <TestComponentWithReverseSpec listener={listener} />;
instance = ReactTestUtils.renderIntoDocument(instance);

expect(listener.mock.calls).toEqual([
['MixinA didMount'],
['MixinBWithReverseSpec didMount'],
['MixinC didMount'],
['Component didMount']
]);
});

it('should validate prop types via mixins', function() {
expect(TestComponent.type.propTypes).toBeDefined();
expect(TestComponent.type.propTypes.value)
Expand Down