Context warning should fire on update, not just initial mount. Renamed a couple instances of context to make it clear if it was masked/unmasked
Context warning should fire on update, not just initial mount. Renamed a couple instances of context to make it clear if it was masked/unmasked
Jim committed
Jan 6, 2015 at 17:38 UTC
e4842b4ae26eff6b70b7bb417744b864ba0b8e62
2 files changed
+211
-1
src/core/ReactCompositeComponent.js
+7
-1
@@ -687,6 +687,12 @@ var ReactCompositeComponentMixin = assign({},
687
nextContext = this._processContext(nextParentElement._context);
688
nextProps = this._processProps(nextParentElement.props);
689
690
+ if (__DEV__) {
691
+ if (nextUnmaskedContext != null) {
692
+ this._warnIfContextsDiffer(nextParentElement._context, nextUnmaskedContext);
693
+ }
694
+ }
695
+
696
this._compositeLifeCycleState = CompositeLifeCycle.RECEIVING_PROPS;
697
if (inst.componentWillReceiveProps) {
698
inst.componentWillReceiveProps(nextProps, nextContext);
@@ -772,7 +778,7 @@ var ReactCompositeComponentMixin = assign({},
778
inst.state = nextState;
779
inst.context = nextContext;
780
775
- this._updateRenderedComponent(transaction, nextContext);
781
+ this._updateRenderedComponent(transaction, unmaskedContext);
782
783
if (inst.componentDidUpdate) {
784
transaction.getReactMountReady().enqueue(
src/core/__tests__/ReactCompositeComponent-test.js
+204
@@ -549,7 +549,14 @@ describe('ReactCompositeComponent', function() {
549
ReactTestUtils.renderIntoDocument(<Component />);
550
});
551
552
+ // Two warnings, one for the component and one for the div
553
+ // We may want to make this expect one warning in the future
554
expect(console.warn.mock.calls.length).toBe(2);
555
+ expect(console.warn.mock.calls[0][0]).toBe(
556
+ 'Warning: owner based context (keys: foo) does not equal parent based ' +
557
+ 'context (keys: ) while mounting Component ' +
558
+ '(see: http://fb.me/react-context-by-parent)'
559
+ );
560
expect(console.warn.mock.calls[1][0]).toBe(
561
'Warning: owner based context (keys: foo) does not equal parent based ' +
562
'context (keys: ) while mounting ReactCompositeComponent ' +
@@ -590,13 +597,210 @@ describe('ReactCompositeComponent', function() {
597
598
ReactTestUtils.renderIntoDocument(<Parent>{component}</Parent>);
599
600
+ // Two warnings, one for the component and one for the div
601
+ // We may want to make this expect one warning in the future
602
+ expect(console.warn.mock.calls.length).toBe(2);
603
+ expect(console.warn.mock.calls[0][0]).toBe(
604
+ 'Warning: owner-based and parent-based contexts differ ' +
605
+ '(values: `noise` vs `bar`) for key (foo) while mounting Component ' +
606
+ '(see: http://fb.me/react-context-by-parent)'
607
+ );
608
+ expect(console.warn.mock.calls[1][0]).toBe(
609
+ 'Warning: owner-based and parent-based contexts differ ' +
610
+ '(values: `noise` vs `bar`) for key (foo) while mounting ReactCompositeComponent ' +
611
+ '(see: http://fb.me/react-context-by-parent)'
612
+ );
613
+
614
+ });
615
+
616
+ it('should warn if context values differ on update using withContext', function() {
617
+ var Parent = React.createClass({
618
+ childContextTypes: {
619
+ foo: ReactPropTypes.string
620
+ },
621
+
622
+ getChildContext: function() {
623
+ return {
624
+ foo: "bar"
625
+ };
626
+ },
627
+
628
+ render: function() {
629
+ return <div>{this.props.children}</div>;
630
+ }
631
+ });
632
+
633
+ var Component = React.createClass({
634
+ contextTypes: {
635
+ foo: ReactPropTypes.string.isRequired
636
+ },
637
+
638
+ render: function() {
639
+ return <div />;
640
+ }
641
+ });
642
+
643
+ var div = document.createElement('div');
644
+
645
+ var componentWithSameContext = React.withContext({foo: 'bar'}, function() {
646
+ return <Component />;
647
+ });
648
+ React.render(<Parent>{componentWithSameContext}</Parent>, div);
649
+
650
+ expect(console.warn.mock.calls.length).toBe(0);
651
+
652
+ var componentWithDifferentContext = React.withContext({foo: 'noise'}, function() {
653
+ return <Component />;
654
+ });
655
+ React.render(<Parent>{componentWithDifferentContext}</Parent>, div);
656
+
657
+ // Two warnings, one for the component and one for the div
658
+ // We may want to make this expect one warning in the future
659
expect(console.warn.mock.calls.length).toBe(2);
660
expect(console.warn.mock.calls[0][0]).toBe(
661
'Warning: owner-based and parent-based contexts differ ' +
662
'(values: `noise` vs `bar`) for key (foo) while mounting Component ' +
663
'(see: http://fb.me/react-context-by-parent)'
664
);
665
+ expect(console.warn.mock.calls[1][0]).toBe(
666
+ 'Warning: owner-based and parent-based contexts differ ' +
667
+ '(values: `noise` vs `bar`) for key (foo) while mounting ReactCompositeComponent ' +
668
+ '(see: http://fb.me/react-context-by-parent)'
669
+ );
670
+
671
+ });
672
+
673
+ it('should warn if context values differ on update using wrapper', function() {
674
+ var Parent = React.createClass({
675
+ childContextTypes: {
676
+ foo: ReactPropTypes.string
677
+ },
678
+
679
+ getChildContext: function() {
680
+ return {
681
+ foo: "bar"
682
+ };
683
+ },
684
+
685
+ render: function() {
686
+ return <div>{this.props.children}</div>;
687
+ }
688
+ });
689
+
690
+ var Component = React.createClass({
691
+ contextTypes: {
692
+ foo: ReactPropTypes.string.isRequired
693
+ },
694
+
695
+ render: function() {
696
+ return <div />;
697
+ }
698
+ });
699
+
700
+ var Wrapper = React.createClass({
701
+ childContextTypes: {
702
+ foo: ReactPropTypes.string
703
+ },
704
+
705
+ getChildContext: function() {
706
+ return { foo: this.props.foo };
707
+ },
708
+
709
+ render: function() { return <Parent><Component /></Parent>; }
710
+
711
+ });
712
+
713
+ var div = document.createElement('div');
714
+ React.render(<Wrapper foo='bar' />, div);
715
+ React.render(<Wrapper foo='noise' />, div);
716
+
717
+ // Two warnings, one for the component and one for the div
718
+ // We may want to make this expect one warning in the future
719
+ expect(console.warn.mock.calls.length).toBe(2);
720
+ expect(console.warn.mock.calls[0][0]).toBe(
721
+ 'Warning: owner-based and parent-based contexts differ ' +
722
+ '(values: `noise` vs `bar`) for key (foo) while mounting Component ' +
723
+ '(see: http://fb.me/react-context-by-parent)'
724
+ );
725
+ expect(console.warn.mock.calls[1][0]).toBe(
726
+ 'Warning: owner-based and parent-based contexts differ ' +
727
+ '(values: `noise` vs `bar`) for key (foo) while mounting ReactCompositeComponent ' +
728
+ '(see: http://fb.me/react-context-by-parent)'
729
+ );
730
+
731
+ });
732
+
733
+ it('unmasked context propagates through updates', function() {
734
+
735
+ var Leaf = React.createClass({
736
+ contextTypes: {
737
+ foo: ReactPropTypes.string.isRequired
738
+ },
739
+
740
+ componentWillReceiveProps: function(nextProps, nextContext) {
741
+ expect('foo' in nextContext).toBe(true);
742
+ },
743
+
744
+ componentDidUpdate: function(prevProps, prevState, prevContext) {
745
+ expect('foo' in prevContext).toBe(true);
746
+ },
747
+
748
+ shouldComponentUpdate: function(nextProps, nextState, nextContext) {
749
+ expect('foo' in nextContext).toBe(true);
750
+ return true;
751
+ },
752
+
753
+ render: function() {
754
+ return <span>{this.context.foo}</span>;
755
+ }
756
+ });
757
+
758
+ var Intermediary = React.createClass({
759
+
760
+ componentWillReceiveProps: function(nextProps, nextContext) {
761
+ expect('foo' in nextContext).toBe(false);
762
+ },
763
+
764
+ componentDidUpdate: function(prevProps, prevState, prevContext) {
765
+ expect('foo' in prevContext).toBe(false);
766
+ },
767
+
768
+ shouldComponentUpdate: function(nextProps, nextState, nextContext) {
769
+ expect('foo' in nextContext).toBe(false);
770
+ return true;
771
+ },
772
+
773
+ render: function() {
774
+ return <Leaf />;
775
+ }
776
+ });
777
+
778
+ var Parent = React.createClass({
779
+ childContextTypes: {
780
+ foo: ReactPropTypes.string
781
+ },
782
+
783
+ getChildContext: function() {
784
+ return {
785
+ foo: this.props.cntxt
786
+ };
787
+ },
788
+
789
+ render: function() {
790
+ return <Intermediary />;
791
+ }
792
+ });
793
794
+ var div = document.createElement('div');
795
+ React.render(<Parent cntxt="noise" />, div);
796
+ expect(div.children[0].innerHTML).toBe('noise');
797
+ div.children[0].innerHTML = 'aliens';
798
+ div.children[0].id = 'aliens';
799
+ expect(div.children[0].innerHTML).toBe('aliens');
800
+ expect(div.children[0].id).toBe('aliens');
801
+ React.render(<Parent cntxt="bar" />, div);
802
+ expect(div.children[0].innerHTML).toBe('bar');
803
+ expect(div.children[0].id).toBe('aliens');
804
});
805
806
it('should disallow nested render calls', function() {