Apply component and mixins specs deterministically Fixes #1589.
Apply component and mixins specs deterministically Fixes #1589.
Dan Abramov committed
May 26, 2014 at 02:29 UTC
367c88e399d60dfa5a5611fcaa5769c18b7ca86e
3 files changed
+49
-2
docs/docs/05-reusable-components.md
+1
-1
@@ -187,5 +187,5 @@ React.renderComponent(
187
);
188
```
189
190
-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.
190
+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.
191
src/core/ReactCompositeComponent.js
+17
-1
@@ -34,6 +34,7 @@ var ReactUpdates = require('ReactUpdates');
34
var instantiateReactComponent = require('instantiateReactComponent');
35
var invariant = require('invariant');
36
var keyMirror = require('keyMirror');
37
+var keyOf = require('keyOf');
38
var merge = require('merge');
39
var mixInto = require('mixInto');
40
var monitorCodeUse = require('monitorCodeUse');
@@ -41,6 +42,8 @@ var mapObject = require('mapObject');
42
var shouldUpdateReactComponent = require('shouldUpdateReactComponent');
43
var warning = require('warning');
44
45
+var MIXINS_KEY = keyOf({mixins: null});
46
+
47
/**
48
* Policies that describe methods in `ReactCompositeComponentInterface`.
49
*/
@@ -453,12 +456,25 @@ function mixSpecIntoComponent(ConvenienceConstructor, spec) {
456
457
var Constructor = ConvenienceConstructor.type;
458
var proto = Constructor.prototype;
459
+
460
+ // By handling mixins before any other properties, we ensure the same
461
+ // chaining order is applied to methods with DEFINE_MANY policy, whether
462
+ // mixins are listed before or after these methods in the spec.
463
+ if (spec.hasOwnProperty(MIXINS_KEY)) {
464
+ RESERVED_SPEC_KEYS.mixins(ConvenienceConstructor, spec.mixins);
465
+ }
466
+
467
for (var name in spec) {
457
- var property = spec[name];
468
if (!spec.hasOwnProperty(name)) {
469
continue;
470
}
471
472
+ if (name === MIXINS_KEY) {
473
+ // We have already handled mixins in a special case above
474
+ continue;
475
+ }
476
+
477
+ var property = spec[name];
478
validateMethodOverride(proto, name);
479
480
if (RESERVED_SPEC_KEYS.hasOwnProperty(name)) {
src/core/__tests__/ReactCompositeComponentMixin-test.js
+31
@@ -27,6 +27,7 @@ var reactComponentExpect;
27
28
var TestComponent;
29
var TestComponentWithPropTypes;
30
+var TestComponentWithReverseSpec;
31
var mixinPropValidator;
32
var componentPropValidator;
33
@@ -58,6 +59,13 @@ describe('ReactCompositeComponent-mixin', function() {
59
}
60
};
61
62
+ var MixinBWithReverseSpec = {
63
+ componentDidMount: function() {
64
+ this.props.listener('MixinBWithReverseSpec didMount');
65
+ },
66
+ mixins: [MixinA]
67
+ };
68
+
69
var MixinC = {
70
statics: {
71
staticC: function() {}
@@ -89,6 +97,16 @@ describe('ReactCompositeComponent-mixin', function() {
97
}
98
});
99
100
+ TestComponentWithReverseSpec = React.createClass({
101
+ render: function() {
102
+ return <div />;
103
+ },
104
+ componentDidMount: function() {
105
+ this.props.listener('Component didMount');
106
+ },
107
+ mixins: [MixinBWithReverseSpec, MixinC, MixinD]
108
+ });
109
+
110
TestComponentWithPropTypes = React.createClass({
111
mixins: [MixinD],
112
propTypes: {
@@ -128,6 +146,19 @@ describe('ReactCompositeComponent-mixin', function() {
146
]);
147
});
148
149
+ it('should chain functions regardless of spec property order', function() {
150
+ var listener = mocks.getMockFunction();
151
+ var instance = <TestComponentWithReverseSpec listener={listener} />;
152
+ instance = ReactTestUtils.renderIntoDocument(instance);
153
+
154
+ expect(listener.mock.calls).toEqual([
155
+ ['MixinA didMount'],
156
+ ['MixinBWithReverseSpec didMount'],
157
+ ['MixinC didMount'],
158
+ ['Component didMount']
159
+ ]);
160
+ });
161
+
162
it('should validate prop types via mixins', function() {
163
expect(TestComponent.type.propTypes).toBeDefined();
164
expect(TestComponent.type.propTypes.value)