@samitouri / QOS-React-2 / commits / 934ef1d4c2

Fix context in callbacks

callbacks like shouldComponentUpdate, componentDidUpdate, etc. were getting the full, unsanitized context, instead of the one that's been filtered down to only the fields allowed to be visible by contextTypes.

Marshall Roch committed Nov 20, 2013 at 14:03 UTC 934ef1d4c223358b987cb2e2121234608d34f1f1
2 files changed +88 -8
src/core/ReactCompositeComponent.js
+16 -8
@@ -219,17 +219,21 @@ var ReactCompositeComponentInterface = {
219
220 /**
221 * Invoked while deciding if the component should be updated as a result of
222 - * receiving new props and state.
222 + * receiving new props, state and/or context.
223 *
224 * Use this as an opportunity to `return false` when you're certain that the
225 - * transition to the new props and state will not require a component update.
225 + * transition to the new props/state/context will not require a component
226 + * update.
227 *
227 - * shouldComponentUpdate: function(nextProps, nextState) {
228 - * return !equal(nextProps, this.props) || !equal(nextState, this.state);
228 + * shouldComponentUpdate: function(nextProps, nextState, nextContext) {
229 + * return !equal(nextProps, this.props) ||
230 + * !equal(nextState, this.state) ||
231 + * !equal(nextContext, this.context);
232 * }
233 *
234 * @param {object} nextProps
235 * @param {?object} nextState
236 + * @param {?object} nextContext
237 * @return {boolean} True if the component should update.
238 * @optional
239 */
@@ -836,7 +840,8 @@ var ReactCompositeComponentMixin = {
840 var nextState = this._pendingState || this.state;
841 this._pendingState = null;
842
839 - var nextContext = this._pendingContext || this._currentContext;
843 + var nextFullContext = this._pendingContext || this._currentContext;
844 + var nextContext = this._processContext(nextFullContext);
845 this._pendingContext = null;
846
847 if (this._pendingForceUpdate ||
@@ -847,6 +852,7 @@ var ReactCompositeComponentMixin = {
852 this._performComponentUpdate(
853 nextProps,
854 nextState,
855 + nextFullContext,
856 nextContext,
857 transaction
858 );
@@ -855,6 +861,7 @@ var ReactCompositeComponentMixin = {
861 // to set props and state.
862 this.props = nextProps;
863 this.state = nextState;
864 + this._currentContext = nextFullContext;
865 this.context = nextContext;
866 }
867
@@ -867,6 +874,7 @@ var ReactCompositeComponentMixin = {
874 *
875 * @param {object} nextProps Next object to set as properties.
876 * @param {?object} nextState Next object to set as state.
877 + * @param {?object} nextFullContext Next object to set as _currentContext.
878 * @param {?object} nextContext Next object to set as context.
879 * @param {ReactReconcileTransaction} transaction
880 * @private
@@ -874,6 +882,7 @@ var ReactCompositeComponentMixin = {
882 _performComponentUpdate: function(
883 nextProps,
884 nextState,
885 + nextFullContext,
886 nextContext,
887 transaction
888 ) {
@@ -887,9 +896,8 @@ var ReactCompositeComponentMixin = {
896
897 this.props = nextProps;
898 this.state = nextState;
890 -
891 - this._currentContext = nextContext;
892 - this.context = this._processContext(nextContext);
899 + this._currentContext = nextFullContext;
900 + this.context = nextContext;
901
902 this.updateComponent(transaction, prevProps, prevState, prevContext);
903
src/core/__tests__/ReactCompositeComponent-test.js
+72
@@ -619,4 +619,76 @@ describe('ReactCompositeComponent', function() {
619 );
620 }).not.toThrow();
621 });
622 +
623 + it('should filter out context not in contextTypes', function() {
624 + var Component = React.createClass({
625 + contextTypes: {
626 + foo: ReactPropTypes.string
627 + },
628 +
629 + render: function() {
630 + return <div />;
631 + }
632 + });
633 +
634 + var instance = React.withContext({foo: 'abc', bar: 123}, function() {
635 + return <Component />;
636 + });
637 + ReactTestUtils.renderIntoDocument(instance);
638 + reactComponentExpect(instance).scalarContextEqual({foo: 'abc'});
639 + });
640 +
641 + it('should filter context properly in callbacks', function() {
642 + var actualShouldComponentUpdate;
643 + var actualComponentWillUpdate;
644 + var actualComponentDidUpdate;
645 +
646 + var Parent = React.createClass({
647 + childContextTypes: {
648 + foo: ReactPropTypes.string.isRequired,
649 + bar: ReactPropTypes.string.isRequired
650 + },
651 +
652 + getChildContext: function() {
653 + return {
654 + foo: this.props.foo,
655 + bar: "bar"
656 + };
657 + },
658 +
659 + render: function() {
660 + return <Component />;
661 + }
662 + });
663 +
664 + var Component = React.createClass({
665 + contextTypes: {
666 + foo: ReactPropTypes.string
667 + },
668 +
669 + shouldComponentUpdate: function(nextProps, nextState, nextContext) {
670 + actualShouldComponentUpdate = nextContext;
671 + return true;
672 + },
673 +
674 + componentWillUpdate: function(nextProps, nextState, nextContext) {
675 + actualComponentWillUpdate = nextContext;
676 + },
677 +
678 + componentDidUpdate: function(prevProps, prevState, prevContext) {
679 + actualComponentDidUpdate = prevContext;
680 + },
681 +
682 + render: function() {
683 + return <div />;
684 + }
685 + });
686 +
687 + var instance = <Parent foo="abc" />;
688 + ReactTestUtils.renderIntoDocument(instance);
689 + instance.replaceProps({foo: "def"});
690 + expect(actualShouldComponentUpdate).toEqual({foo: 'def'});
691 + expect(actualComponentWillUpdate).toEqual({foo: 'def'});
692 + expect(actualComponentDidUpdate).toEqual({foo: 'abc'});
693 + });
694 });