From 71e49adf35a19d57b4937f429e0902253edd7dfc Mon Sep 17 00:00:00 2001 From: Charles Marsh Date: Wed, 13 Aug 2014 14:44:54 -0400 Subject: [PATCH 1/2] Throw an error when functions on `statics` clash due to duplicate keys --- src/core/ReactCompositeComponent.js | 24 +++++---------- .../__tests__/ReactCompositeComponent-test.js | 30 +++++++++++++++++-- 2 files changed, 35 insertions(+), 19 deletions(-) diff --git a/src/core/ReactCompositeComponent.js b/src/core/ReactCompositeComponent.js index 42a1dcb45b5..5c1e0656652 100644 --- a/src/core/ReactCompositeComponent.js +++ b/src/core/ReactCompositeComponent.js @@ -556,22 +556,14 @@ function mixStaticSpecIntoComponent(Constructor, statics) { } 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; } } diff --git a/src/core/__tests__/ReactCompositeComponent-test.js b/src/core/__tests__/ReactCompositeComponent-test.js index abfedc5dee7..2deb31e912a 100644 --- a/src/core/__tests__/ReactCompositeComponent-test.js +++ b/src/core/__tests__/ReactCompositeComponent-test.js @@ -1317,9 +1317,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 ; + } + }); + }).toThrow( + 'Invariant Violation: ReactCompositeComponent: You are attempting to ' + + 'define `abc` on your component more than once. This conflict may be ' + + 'due to a mixin.' ); }); From a70db003dbdba83f04e2480e4a97cc9fbc7d147c Mon Sep 17 00:00:00 2001 From: Charles Marsh Date: Wed, 13 Aug 2014 21:34:02 -0400 Subject: [PATCH 2/2] Throw if a reserved property is defined in 'statics' --- src/core/ReactCompositeComponent.js | 10 ++++++++ .../__tests__/ReactCompositeComponent-test.js | 23 +++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/src/core/ReactCompositeComponent.js b/src/core/ReactCompositeComponent.js index 5c1e0656652..42872293d0e 100644 --- a/src/core/ReactCompositeComponent.js +++ b/src/core/ReactCompositeComponent.js @@ -555,6 +555,16 @@ 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; invariant( !isInherited, diff --git a/src/core/__tests__/ReactCompositeComponent-test.js b/src/core/__tests__/ReactCompositeComponent-test.js index 2deb31e912a..20c7accb698 100644 --- a/src/core/__tests__/ReactCompositeComponent-test.js +++ b/src/core/__tests__/ReactCompositeComponent-test.js @@ -1272,6 +1272,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 ; + } + }); + }).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: {