Merge pull request #2036 from crm416/static-functions

Throw an error when functions on `statics` clash due to duplicate keys

Author: chenglou

src/core/ReactCompositeComponent.js

@@ -575,23 +575,25 @@ function mixStaticSpecIntoComponent(Constructor, statics) {
       continue;
     }
 
+    var isReserved = name in RESERVED_SPEC_KEYS;
+    invariant(
+      !isReserved,
+      'ReactCompositeComponent: You are attempting to define a reserved ' +
+      'property, `%s`, that shouldn\'t be on the "statics" key. Define it ' +
+      'as an instance property instead; it will still be accessible on the ' +
+      'constructor.',
+      name
+    );
+
     var isInherited = name in Constructor;
-    var result = property;
-    if (isInherited) {
-      var existingProperty = Constructor[name];
-      var existingType = typeof existingProperty;
-      var propertyType = typeof property;
-      invariant(
-        existingType === 'function' && propertyType === 'function',
-        'ReactCompositeComponent: You are attempting to define ' +
-        '`%s` on your component more than once, but that is only supported ' +
-        'for functions, which are chained together. This conflict may be ' +
-        'due to a mixin.',
-        name
-      );
-      result = createChainedFunction(existingProperty, property);
-    }
-    Constructor[name] = result;
+    invariant(
+      !isInherited,
+      'ReactCompositeComponent: You are attempting to define ' +
+      '`%s` on your component more than once. This conflict may be ' +
+      'due to a mixin.',
+      name
+    );
+    Constructor[name] = property;
   }
 }
 

src/core/__tests__/ReactCompositeComponent-test.js

@@ -1276,6 +1276,29 @@ describe('ReactCompositeComponent', function() {
     expect(Component.pqr()).toBe(Component.type);
   });
 
+  it('should throw if a reserved property is in statics', function() {
+    expect(function() {
+      React.createClass({
+        statics: {
+          getDefaultProps: function() {
+            return {
+              foo: 0
+            };
+          }
+        },
+
+        render: function() {
+          return <span />;
+        }
+      });
+    }).toThrow(
+      'Invariant Violation: ReactCompositeComponent: You are attempting to ' +
+      'define a reserved property, `getDefaultProps`, that shouldn\'t be on ' +
+      'the "statics" key. Define it as an instance property instead; it ' +
+      'will still be accessible on the constructor.'
+    );
+  });
+
   it('should support statics in mixins', function() {
     var Mixin = {
       statics: {
@@ -1321,9 +1344,33 @@ describe('ReactCompositeComponent', function() {
       });
     }).toThrow(
       'Invariant Violation: ReactCompositeComponent: You are attempting to ' +
-      'define `abc` on your component more than once, but that is only ' +
-      'supported for functions, which are chained together. This conflict ' +
-      'may be due to a mixin.'
+      'define `abc` on your component more than once. This conflict may be ' +
+      'due to a mixin.'
+    );
+  });
+
+  it("should throw if mixins override functions in statics", function() {
+    expect(function() {
+      var Mixin = {
+        statics: {
+          abc: function() { console.log('foo'); }
+        }
+      };
+      React.createClass({
+        mixins: [Mixin],
+
+        statics: {
+          abc: function() { console.log('bar'); }
+        },
+
+        render: function() {
+          return <span />;
+        }
+      });
+    }).toThrow(
+      'Invariant Violation: ReactCompositeComponent: You are attempting to ' +
+      'define `abc` on your component more than once. This conflict may be ' +
+      'due to a mixin.'
     );
   });