@samitouri / QOS-React-2 / commits / 26d7c4275a

Add warning when using componentShouldUpdate

Christopher Chedeau committed Oct 7, 2013 at 14:54 UTC 26d7c4275a09c8d735abc9a3bad7379f561973ef
2 files changed +61 -2
src/core/ReactCompositeComponent.js
+13
@@ -957,10 +957,23 @@ var ReactCompositeComponent = {
957 Constructor.prototype = new ReactCompositeComponentBase();
958 Constructor.prototype.constructor = Constructor;
959 mixSpecIntoComponent(Constructor, spec);
960 +
961 invariant(
962 Constructor.prototype.render,
963 'createClass(...): Class specification must implement a `render` method.'
964 );
965 +
966 + if (__DEV__) {
967 + if (Constructor.prototype.componentShouldUpdate) {
968 + console.warn(
969 + (spec.displayName || 'A component') + ' has a method called ' +
970 + 'componentShouldUpdate(). Did you mean shouldComponentUpdate()? ' +
971 + 'The name is phrased as a question because the function is ' +
972 + 'expected to return a value.'
973 + );
974 + }
975 + }
976 +
977 // Reduce time spent doing lookups by setting these on the prototype.
978 for (var methodName in ReactCompositeComponentInterface) {
979 if (!Constructor.prototype[methodName]) {
src/core/__tests__/ReactCompositeComponent-test.js
+48 -2
@@ -30,11 +30,14 @@ var ReactDoNotBindDeprecated;
30
31 var cx;
32 var reactComponentExpect;
33 +var mocks;
34
35 describe('ReactCompositeComponent', function() {
36
37 beforeEach(function() {
38 cx = require('cx');
39 + mocks = require('mocks');
40 +
41 reactComponentExpect = require('reactComponentExpect');
42 React = require('React');
43 ReactCurrentOwner = require('ReactCurrentOwner');
@@ -394,17 +397,60 @@ describe('ReactCompositeComponent', function() {
397 it('should detect invalid CompositeComponent classes', function() {
398 var FnComponent = function() {
399 return false;
397 - }
400 + };
401
402 var NullComponent = null;
403
404 var TrickFnComponent = function() {
405 return true;
403 - }
406 + };
407 TrickFnComponent.componentConstructor = true;
408
409 expect(React.isValidClass(FnComponent)).toBe(false);
410 expect(React.isValidClass(NullComponent)).toBe(false);
411 expect(React.isValidClass(TrickFnComponent)).toBe(false);
412 });
413 +
414 + it('should warn when mispelling shouldComponentUpdate', function() {
415 + var warn = console.warn;
416 + console.warn = mocks.getMockFunction();
417 +
418 + try {
419 + React.createClass({
420 + componentShouldUpdate: function() {
421 + return false;
422 + },
423 + render: function() {
424 + return <div />;
425 + }
426 + });
427 + expect(console.warn.mock.calls.length).toBe(1);
428 + expect(console.warn.mock.calls[0][0]).toBe(
429 + 'A component has a method called componentShouldUpdate(). Did you ' +
430 + 'mean shouldComponentUpdate()? The name is phrased as a question ' +
431 + 'because the function is expected to return a value.'
432 + );
433 +
434 + var NamedComponent = React.createClass({
435 + componentShouldUpdate: function() {
436 + return false;
437 + },
438 + render: function() {
439 + return <div />;
440 + }
441 + });
442 + expect(console.warn.mock.calls.length).toBe(2);
443 + expect(console.warn.mock.calls[1][0]).toBe(
444 + 'NamedComponent has a method called componentShouldUpdate(). Did you ' +
445 + 'mean shouldComponentUpdate()? The name is phrased as a question ' +
446 + 'because the function is expected to return a value.'
447 + );
448 +
449 + NamedComponent(); // Shut up lint
450 + } catch (e) {
451 + throw e;
452 + } finally {
453 + console.warn = warn;
454 + }
455 + });
456 });