@samitouri / QOS-React / commits / 4482fddeda

Fix host context issues around EventComponents and EventTargets (#15284)

Dominic Gannaway committed Apr 1, 2019 at 19:33 UTC 4482fddeda166b0ce4ff3e4f1d8f8b6d9b26179c
3 files changed +551 -1
packages/react-reconciler/src/ReactFiberBeginWork.js
+8 -1
@@ -2112,8 +2112,15 @@ function beginWork(
2112 // been unsuspended it has committed as a regular Suspense component.
2113 // If it needs to be retried, it should have work scheduled on it.
2114 workInProgress.effectTag |= DidCapture;
2115 - break;
2115 }
2116 + break;
2117 + }
2118 + case EventComponent:
2119 + case EventTarget: {
2120 + if (enableEventAPI) {
2121 + pushHostContextForEvent(workInProgress);
2122 + }
2123 + break;
2124 }
2125 }
2126 return bailoutOnAlreadyFinishedWork(
packages/react-reconciler/src/ReactFiberUnwindWork.js
+9
@@ -27,6 +27,8 @@ import {
27 SuspenseComponent,
28 DehydratedSuspenseComponent,
29 IncompleteClassComponent,
30 + EventComponent,
31 + EventTarget,
32 } from 'shared/ReactWorkTags';
33 import {
34 DidCapture,
@@ -38,6 +40,7 @@ import {
40 import {
41 enableSchedulerTracing,
42 enableSuspenseServerRenderer,
43 + enableEventAPI,
44 } from 'shared/ReactFeatureFlags';
45 import {ConcurrentMode} from './ReactTypeOfMode';
46 import {shouldCaptureSuspense} from './ReactFiberSuspenseComponent';
@@ -510,6 +513,12 @@ function unwindWork(
513 case ContextProvider:
514 popProvider(workInProgress);
515 return null;
516 + case EventComponent:
517 + case EventTarget:
518 + if (enableEventAPI) {
519 + popHostContext(workInProgress);
520 + }
521 + return null;
522 default:
523 return null;
524 }
packages/react-reconciler/src/__tests__/ReactFiberEvents-test-internal.js
+534
@@ -17,6 +17,7 @@ let EventComponent;
17 let ReactTestRenderer;
18 let ReactDOM;
19 let ReactDOMServer;
20 +let ReactTestUtils;
21 let EventTarget;
22 let ReactEvents;
23
@@ -55,6 +56,7 @@ function initTestRenderer() {
56 function initReactDOM() {
57 init();
58 ReactDOM = require('react-dom');
59 + ReactTestUtils = require('react-dom/test-utils');
60 }
61
62 function initReactDOMServer() {
@@ -219,6 +221,188 @@ describe('ReactFiberEvents', () => {
221 'Warning: validateDOMNesting: React event targets must not have event components as children.',
222 );
223 });
224 +
225 + it('should handle event components correctly with error boundaries', () => {
226 + function ErrorComponent() {
227 + throw new Error('Failed!');
228 + }
229 +
230 + const Test = () => (
231 + <EventComponent>
232 + <EventTarget>
233 + <span>
234 + <ErrorComponent />
235 + </span>
236 + </EventTarget>
237 + </EventComponent>
238 + );
239 +
240 + class Wrapper extends React.Component {
241 + state = {
242 + error: null,
243 + };
244 +
245 + componentDidCatch(error) {
246 + this.setState({
247 + error,
248 + });
249 + }
250 +
251 + render() {
252 + if (this.state.error) {
253 + return 'Worked!';
254 + }
255 + return <Test />;
256 + }
257 + }
258 +
259 + ReactNoop.render(<Wrapper />);
260 + expect(Scheduler).toFlushWithoutYielding();
261 + expect(ReactNoop).toMatchRenderedOutput('Worked!');
262 + });
263 +
264 + it('should handle re-renders where there is a bail-out in a parent', () => {
265 + let _updateCounter;
266 +
267 + function Child() {
268 + const [counter, updateCounter] = React.useState(0);
269 +
270 + _updateCounter = updateCounter;
271 +
272 + return (
273 + <div>
274 + <span>Child - {counter}</span>
275 + </div>
276 + );
277 + }
278 +
279 + const Parent = () => (
280 + <EventComponent>
281 + <EventTarget>
282 + <div>
283 + <Child />
284 + </div>
285 + </EventTarget>
286 + </EventComponent>
287 + );
288 +
289 + ReactNoop.render(<Parent />);
290 + expect(Scheduler).toFlushWithoutYielding();
291 + expect(ReactNoop).toMatchRenderedOutput(
292 + <div>
293 + <div>
294 + <span>Child - 0</span>
295 + </div>
296 + </div>,
297 + );
298 +
299 + ReactNoop.act(() => {
300 + _updateCounter(counter => counter + 1);
301 + });
302 + expect(Scheduler).toFlushWithoutYielding();
303 +
304 + expect(ReactNoop).toMatchRenderedOutput(
305 + <div>
306 + <div>
307 + <span>Child - 1</span>
308 + </div>
309 + </div>,
310 + );
311 + });
312 +
313 + it('should handle re-renders where there is a bail-out in a parent and an error occurs', () => {
314 + let _updateCounter;
315 +
316 + function Child() {
317 + const [counter, updateCounter] = React.useState(0);
318 +
319 + _updateCounter = updateCounter;
320 +
321 + if (counter === 1) {
322 + return null;
323 + }
324 +
325 + return (
326 + <div>
327 + <span>Child - {counter}</span>
328 + </div>
329 + );
330 + }
331 +
332 + const Parent = () => (
333 + <EventComponent>
334 + <EventTarget>
335 + <Child />
336 + </EventTarget>
337 + </EventComponent>
338 + );
339 +
340 + ReactNoop.render(<Parent />);
341 + expect(Scheduler).toFlushWithoutYielding();
342 + expect(ReactNoop).toMatchRenderedOutput(
343 + <div>
344 + <span>Child - 0</span>
345 + </div>,
346 + );
347 +
348 + expect(() => {
349 + ReactNoop.act(() => {
350 + _updateCounter(counter => counter + 1);
351 + });
352 + expect(Scheduler).toFlushWithoutYielding();
353 + }).toWarnDev(
354 + 'Warning: <TouchHitTarget> must have a single DOM element as a child. Found no children.',
355 + );
356 + });
357 +
358 + it('should handle re-renders where there is a bail-out in a parent and an error occurs #2', () => {
359 + let _updateCounter;
360 +
361 + function Child() {
362 + const [counter, updateCounter] = React.useState(0);
363 +
364 + _updateCounter = updateCounter;
365 +
366 + if (counter === 1) {
367 + return (
368 + <EventComponent>
369 + <div>Child</div>
370 + </EventComponent>
371 + );
372 + }
373 +
374 + return (
375 + <div>
376 + <span>Child - {counter}</span>
377 + </div>
378 + );
379 + }
380 +
381 + const Parent = () => (
382 + <EventComponent>
383 + <EventTarget>
384 + <Child />
385 + </EventTarget>
386 + </EventComponent>
387 + );
388 +
389 + ReactNoop.render(<Parent />);
390 + expect(Scheduler).toFlushWithoutYielding();
391 + expect(ReactNoop).toMatchRenderedOutput(
392 + <div>
393 + <span>Child - 0</span>
394 + </div>,
395 + );
396 +
397 + expect(() => {
398 + ReactNoop.act(() => {
399 + _updateCounter(counter => counter + 1);
400 + });
401 + expect(Scheduler).toFlushWithoutYielding();
402 + }).toWarnDev(
403 + 'Warning: validateDOMNesting: React event targets must not have event components as children.',
404 + );
405 + });
406 });
407
408 describe('TestRenderer', () => {
@@ -407,6 +591,190 @@ describe('ReactFiberEvents', () => {
591 'Warning: validateDOMNesting: React event targets must not have event components as children.',
592 );
593 });
594 +
595 + it('should handle event components correctly with error boundaries', () => {
596 + function ErrorComponent() {
597 + throw new Error('Failed!');
598 + }
599 +
600 + const Test = () => (
601 + <EventComponent>
602 + <EventTarget>
603 + <span>
604 + <ErrorComponent />
605 + </span>
606 + </EventTarget>
607 + </EventComponent>
608 + );
609 +
610 + class Wrapper extends React.Component {
611 + state = {
612 + error: null,
613 + };
614 +
615 + componentDidCatch(error) {
616 + this.setState({
617 + error,
618 + });
619 + }
620 +
621 + render() {
622 + if (this.state.error) {
623 + return 'Worked!';
624 + }
625 + return <Test />;
626 + }
627 + }
628 +
629 + const root = ReactTestRenderer.create(null);
630 + root.update(<Wrapper />);
631 + expect(Scheduler).toFlushWithoutYielding();
632 + expect(root).toMatchRenderedOutput('Worked!');
633 + });
634 +
635 + it('should handle re-renders where there is a bail-out in a parent', () => {
636 + let _updateCounter;
637 +
638 + function Child() {
639 + const [counter, updateCounter] = React.useState(0);
640 +
641 + _updateCounter = updateCounter;
642 +
643 + return (
644 + <div>
645 + <span>Child - {counter}</span>
646 + </div>
647 + );
648 + }
649 +
650 + const Parent = () => (
651 + <EventComponent>
652 + <EventTarget>
653 + <div>
654 + <Child />
655 + </div>
656 + </EventTarget>
657 + </EventComponent>
658 + );
659 +
660 + const root = ReactTestRenderer.create(null);
661 + root.update(<Parent />);
662 + expect(Scheduler).toFlushWithoutYielding();
663 + expect(root).toMatchRenderedOutput(
664 + <div>
665 + <div>
666 + <span>Child - 0</span>
667 + </div>
668 + </div>,
669 + );
670 +
671 + ReactTestRenderer.act(() => {
672 + _updateCounter(counter => counter + 1);
673 + });
674 +
675 + expect(root).toMatchRenderedOutput(
676 + <div>
677 + <div>
678 + <span>Child - 1</span>
679 + </div>
680 + </div>,
681 + );
682 + });
683 +
684 + it('should handle re-renders where there is a bail-out in a parent and an error occurs', () => {
685 + let _updateCounter;
686 +
687 + function Child() {
688 + const [counter, updateCounter] = React.useState(0);
689 +
690 + _updateCounter = updateCounter;
691 +
692 + if (counter === 1) {
693 + return null;
694 + }
695 +
696 + return (
697 + <div>
698 + <span>Child - {counter}</span>
699 + </div>
700 + );
701 + }
702 +
703 + const Parent = () => (
704 + <EventComponent>
705 + <EventTarget>
706 + <Child />
707 + </EventTarget>
708 + </EventComponent>
709 + );
710 +
711 + const root = ReactTestRenderer.create(null);
712 + root.update(<Parent />);
713 + expect(Scheduler).toFlushWithoutYielding();
714 + expect(root).toMatchRenderedOutput(
715 + <div>
716 + <span>Child - 0</span>
717 + </div>,
718 + );
719 +
720 + expect(() => {
721 + ReactTestRenderer.act(() => {
722 + _updateCounter(counter => counter + 1);
723 + });
724 + }).toWarnDev(
725 + 'Warning: <TouchHitTarget> must have a single DOM element as a child. Found no children.',
726 + );
727 + });
728 +
729 + it('should handle re-renders where there is a bail-out in a parent and an error occurs #2', () => {
730 + let _updateCounter;
731 +
732 + function Child() {
733 + const [counter, updateCounter] = React.useState(0);
734 +
735 + _updateCounter = updateCounter;
736 +
737 + if (counter === 1) {
738 + return (
739 + <EventComponent>
740 + <div>Child</div>
741 + </EventComponent>
742 + );
743 + }
744 +
745 + return (
746 + <div>
747 + <span>Child - {counter}</span>
748 + </div>
749 + );
750 + }
751 +
752 + const Parent = () => (
753 + <EventComponent>
754 + <EventTarget>
755 + <Child />
756 + </EventTarget>
757 + </EventComponent>
758 + );
759 +
760 + const root = ReactTestRenderer.create(null);
761 + root.update(<Parent />);
762 + expect(Scheduler).toFlushWithoutYielding();
763 + expect(root).toMatchRenderedOutput(
764 + <div>
765 + <span>Child - 0</span>
766 + </div>,
767 + );
768 +
769 + expect(() => {
770 + ReactTestRenderer.act(() => {
771 + _updateCounter(counter => counter + 1);
772 + });
773 + expect(Scheduler).toFlushWithoutYielding();
774 + }).toWarnDev(
775 + 'Warning: validateDOMNesting: React event targets must not have event components as children.',
776 + );
777 + });
778 });
779
780 describe('ReactDOM', () => {
@@ -594,6 +962,172 @@ describe('ReactFiberEvents', () => {
962 'Warning: validateDOMNesting: React event targets must not have event components as children.',
963 );
964 });
965 +
966 + it('should handle event components correctly with error boundaries', () => {
967 + function ErrorComponent() {
968 + throw new Error('Failed!');
969 + }
970 +
971 + const Test = () => (
972 + <EventComponent>
973 + <EventTarget>
974 + <span>
975 + <ErrorComponent />
976 + </span>
977 + </EventTarget>
978 + </EventComponent>
979 + );
980 +
981 + class Wrapper extends React.Component {
982 + state = {
983 + error: null,
984 + };
985 +
986 + componentDidCatch(error) {
987 + this.setState({
988 + error,
989 + });
990 + }
991 +
992 + render() {
993 + if (this.state.error) {
994 + return 'Worked!';
995 + }
996 + return <Test />;
997 + }
998 + }
999 +
1000 + const container = document.createElement('div');
1001 + ReactDOM.render(<Wrapper />, container);
1002 + expect(Scheduler).toFlushWithoutYielding();
1003 + expect(container.innerHTML).toBe('Worked!');
1004 + });
1005 +
1006 + it('should handle re-renders where there is a bail-out in a parent', () => {
1007 + let _updateCounter;
1008 +
1009 + function Child() {
1010 + const [counter, updateCounter] = React.useState(0);
1011 +
1012 + _updateCounter = updateCounter;
1013 +
1014 + return (
1015 + <div>
1016 + <span>Child - {counter}</span>
1017 + </div>
1018 + );
1019 + }
1020 +
1021 + const Parent = () => (
1022 + <EventComponent>
1023 + <EventTarget>
1024 + <div>
1025 + <Child />
1026 + </div>
1027 + </EventTarget>
1028 + </EventComponent>
1029 + );
1030 +
1031 + const container = document.createElement('div');
1032 + ReactDOM.render(<Parent />, container);
1033 + expect(container.innerHTML).toBe(
1034 + '<div><div><span>Child - 0</span></div></div>',
1035 + );
1036 +
1037 + ReactTestUtils.act(() => {
1038 + _updateCounter(counter => counter + 1);
1039 + });
1040 +
1041 + expect(container.innerHTML).toBe(
1042 + '<div><div><span>Child - 1</span></div></div>',
1043 + );
1044 + });
1045 +
1046 + it('should handle re-renders where there is a bail-out in a parent and an error occurs', () => {
1047 + let _updateCounter;
1048 +
1049 + function Child() {
1050 + const [counter, updateCounter] = React.useState(0);
1051 +
1052 + _updateCounter = updateCounter;
1053 +
1054 + if (counter === 1) {
1055 + return null;
1056 + }
1057 +
1058 + return (
1059 + <div>
1060 + <span>Child - {counter}</span>
1061 + </div>
1062 + );
1063 + }
1064 +
1065 + const Parent = () => (
1066 + <EventComponent>
1067 + <EventTarget>
1068 + <Child />
1069 + </EventTarget>
1070 + </EventComponent>
1071 + );
1072 +
1073 + const container = document.createElement('div');
1074 + ReactDOM.render(<Parent />, container);
1075 + expect(container.innerHTML).toBe('<div><span>Child - 0</span></div>');
1076 +
1077 + expect(() => {
1078 + ReactTestUtils.act(() => {
1079 + _updateCounter(counter => counter + 1);
1080 + });
1081 + expect(Scheduler).toFlushWithoutYielding();
1082 + }).toWarnDev(
1083 + 'Warning: <TouchHitTarget> must have a single DOM element as a child. Found no children.',
1084 + );
1085 + });
1086 +
1087 + it('should handle re-renders where there is a bail-out in a parent and an error occurs #2', () => {
1088 + let _updateCounter;
1089 +
1090 + function Child() {
1091 + const [counter, updateCounter] = React.useState(0);
1092 +
1093 + _updateCounter = updateCounter;
1094 +
1095 + if (counter === 1) {
1096 + return (
1097 + <EventComponent>
1098 + <div>Child</div>
1099 + </EventComponent>
1100 + );
1101 + }
1102 +
1103 + return (
1104 + <div>
1105 + <span>Child - {counter}</span>
1106 + </div>
1107 + );
1108 + }
1109 +
1110 + const Parent = () => (
1111 + <EventComponent>
1112 + <EventTarget>
1113 + <Child />
1114 + </EventTarget>
1115 + </EventComponent>
1116 + );
1117 +
1118 + const container = document.createElement('div');
1119 + ReactDOM.render(<Parent />, container);
1120 + expect(container.innerHTML).toBe('<div><span>Child - 0</span></div>');
1121 +
1122 + expect(() => {
1123 + ReactTestUtils.act(() => {
1124 + _updateCounter(counter => counter + 1);
1125 + });
1126 + expect(Scheduler).toFlushWithoutYielding();
1127 + }).toWarnDev(
1128 + 'Warning: validateDOMNesting: React event targets must not have event components as children.',
1129 + );
1130 + });
1131 });
1132
1133 describe('ReactDOMServer', () => {