Merge pull request facebook#3675 from spicyj/facebookgh-3655

Add warning for getDefaultProps on ES6 classes
zpao · Apr 18, 2015 · 4cecb72 · 4cecb72
1 parent db6dcd6
commit 4cecb72
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 6 deletions.
diff --git a/src/core/ReactCompositeComponent.js b/src/core/ReactCompositeComponent.js
@@ -174,6 +174,14 @@ var ReactCompositeComponentMixin = {
         'Did you mean to define a state property instead?',
         this.getName() || 'a component'
       );
+      warning(
+        !inst.getDefaultProps ||
+        inst.getDefaultProps.isReactClassApproved,
+        'getDefaultProps was defined on %s, a plain JavaScript class. ' +
+        'This is only supported for classes created using React.createClass. ' +
+        'Use a static property to define defaultProps instead.',
+        this.getName() || 'a component'
+      );
       warning(
         !inst.propTypes,
         'propTypes was defined as an instance property on %s. Use a static ' +

diff --git a/src/modern/class/__tests__/ReactCoffeeScriptClass-test.coffee b/src/modern/class/__tests__/ReactCoffeeScriptClass-test.coffee
@@ -266,6 +266,7 @@ describe 'ReactCoffeeScriptClass', ->
       but does not invoke them.', ->
     spyOn console, 'warn'
     getInitialStateWasCalled = false
+    getDefaultPropsWasCalled = false
     class Foo extends React.Component
       constructor: ->
         @contextTypes = {}
@@ -275,20 +276,28 @@ describe 'ReactCoffeeScriptClass', ->
         getInitialStateWasCalled = true
         {}
 
+      getDefaultProps: ->
+        getDefaultPropsWasCalled = true
+        {}
+
       render: ->
         span
           className: 'foo'
 
     test React.createElement(Foo), 'SPAN', 'foo'
     expect(getInitialStateWasCalled).toBe false
-    expect(console.warn.calls.length).toBe 3
+    expect(getDefaultPropsWasCalled).toBe false
+    expect(console.warn.calls.length).toBe 4
     expect(console.warn.calls[0].args[0]).toContain(
       'getInitialState was defined on Foo, a plain JavaScript class.'
     )
     expect(console.warn.calls[1].args[0]).toContain(
-      'propTypes was defined as an instance property on Foo.'
+      'getDefaultProps was defined on Foo, a plain JavaScript class.'
     )
     expect(console.warn.calls[2].args[0]).toContain(
+      'propTypes was defined as an instance property on Foo.'
+    )
+    expect(console.warn.calls[3].args[0]).toContain(
       'contextTypes was defined as an instance property on Foo.'
     )
 

diff --git a/src/modern/class/__tests__/ReactES6Class-test.js b/src/modern/class/__tests__/ReactES6Class-test.js
@@ -297,6 +297,7 @@ describe('ReactES6Class', function() {
   it('warns when classic properties are defined on the instance, ' +
      'but does not invoke them.', function() {
     spyOn(console, 'warn');
+    var getDefaultPropsWasCalled = false;
     var getInitialStateWasCalled = false;
     class Foo extends React.Component {
       constructor() {
@@ -307,20 +308,28 @@ describe('ReactES6Class', function() {
         getInitialStateWasCalled = true;
         return {};
       }
+      getDefaultProps() {
+        getDefaultPropsWasCalled = true;
+        return {};
+      }
       render() {
         return <span className="foo" />;
       }
     }
     test(<Foo />, 'SPAN', 'foo');
     expect(getInitialStateWasCalled).toBe(false);
-    expect(console.warn.calls.length).toBe(3);
+    expect(getDefaultPropsWasCalled).toBe(false);
+    expect(console.warn.calls.length).toBe(4);
     expect(console.warn.calls[0].args[0]).toContain(
       'getInitialState was defined on Foo, a plain JavaScript class.'
     );
     expect(console.warn.calls[1].args[0]).toContain(
-      'propTypes was defined as an instance property on Foo.'
+      'getDefaultProps was defined on Foo, a plain JavaScript class.'
     );
     expect(console.warn.calls[2].args[0]).toContain(
+      'propTypes was defined as an instance property on Foo.'
+    );
+    expect(console.warn.calls[3].args[0]).toContain(
       'contextTypes was defined as an instance property on Foo.'
     );
   });

diff --git a/src/modern/class/__tests__/ReactTypeScriptClass-test.ts b/src/modern/class/__tests__/ReactTypeScriptClass-test.ts
@@ -235,9 +235,14 @@ class NormalLifeCycles {
 // warns when classic properties are defined on the instance,
 // but does not invoke them.
 var getInitialStateWasCalled = false;
+var getDefaultPropsWasCalled = false;
 class ClassicProperties extends React.Component {
   contextTypes = {};
   propTypes = {};
+  getDefaultProps() {
+    getDefaultPropsWasCalled = true;
+    return {};
+  }
   getInitialState() {
     getInitialStateWasCalled = true;
     return {};
@@ -410,17 +415,23 @@ describe('ReactTypeScriptClass', function() {
     var warn = jest.genMockFn();
     console.warn = warn;
     getInitialStateWasCalled = false;
+    getDefaultPropsWasCalled = false;
     test(React.createElement(ClassicProperties), 'SPAN', 'foo');
     expect(getInitialStateWasCalled).toBe(false);
-    expect(warn.mock.calls.length).toBe(3);
+    expect(getDefaultPropsWasCalled).toBe(false);
+    expect(warn.mock.calls.length).toBe(4);
     expect(warn.mock.calls[0][0]).toContain(
       'getInitialState was defined on ClassicProperties, ' +
       'a plain JavaScript class.'
     );
     expect(warn.mock.calls[1][0]).toContain(
-      'propTypes was defined as an instance property on ClassicProperties.'
+      'getDefaultProps was defined on ClassicProperties, ' +
+      'a plain JavaScript class.'
     );
     expect(warn.mock.calls[2][0]).toContain(
+      'propTypes was defined as an instance property on ClassicProperties.'
+    );
+    expect(warn.mock.calls[3][0]).toContain(
       'contextTypes was defined as an instance property on ClassicProperties.'
     );
   });