@samitouri / QOS-React / commits / 99cae887f3

Add failing test for passive effect cleanup functions and memoized components (#19750)

* Add failing tests for passive effects cleanup not being called for memoized components * Bubble passive static subtreeTag even after bailout This prevents subsequent unmounts from skipping over any pending passive effect destroy functions

Brian Vaughn committed Sep 2, 2020 at 13:01 UTC 99cae887f3a8bde760a111516d254c1225242edf

2 files changed +156

packages/react-reconciler/src/ReactFiberWorkLoop.new.js

+18

@@ -1904,6 +1904,14 @@ function resetChildLanes(completedWork: Fiber) {

1904 mergeLanes(child.lanes, child.childLanes),

1905 );

1906

1907 + // Preserve passive static flag even in the case of a bailout;

1908 + // otherwise a subsequent unmount may bailout before calling destroy functions.

1909 + subtreeTag |= child.subtreeTag & PassiveStaticSubtreeTag;

1910 + const effectTag = child.effectTag;

1911 + if ((effectTag & PassiveStatic) !== NoEffect) {

1912 + subtreeTag |= PassiveStaticSubtreeTag;

1913 + }

1914 +

1915 treeBaseDuration += child.treeBaseDuration;

1916 child = child.sibling;

1917 }

@@ -1928,9 +1936,19 @@ function resetChildLanes(completedWork: Fiber) {

1936 mergeLanes(child.lanes, child.childLanes),

1937 );

1938

1939 + // Preserve passive static flag even in the case of a bailout;

1940 + // otherwise a subsequent unmount may bailout before calling destroy functions.

1941 + subtreeTag |= child.subtreeTag & PassiveStaticSubtreeTag;

1942 + const effectTag = child.effectTag;

1943 + if ((effectTag & PassiveStatic) !== NoEffect) {

1944 + subtreeTag |= PassiveStaticSubtreeTag;

1945 + }

1946 +

1947 child = child.sibling;

1948 }

1949 }

1950 +

1951 + completedWork.subtreeTag |= subtreeTag;

1952 }

1953

1954 completedWork.childLanes = newChildLanes;

packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js

+138

@@ -2734,6 +2734,144 @@ describe('ReactHooksWithNoopRenderer', () => {

2734 expect(ReactNoop.getChildren()).toEqual([]);

2735 });

2736 });

2737 +

2738 + it('calls passive effect destroy functions for memoized components', () => {

2739 + const Wrapper = ({children}) => children;

2740 + function Child() {

2741 + React.useEffect(() => {

2742 + Scheduler.unstable_yieldValue('passive create');

2743 + return () => {

2744 + Scheduler.unstable_yieldValue('passive destroy');

2745 + };

2746 + }, []);

2747 + React.useLayoutEffect(() => {

2748 + Scheduler.unstable_yieldValue('layout create');

2749 + return () => {

2750 + Scheduler.unstable_yieldValue('layout destroy');

2751 + };

2752 + }, []);

2753 + Scheduler.unstable_yieldValue('render');

2754 + return null;

2755 + }

2756 +

2757 + const isEqual = (prevProps, nextProps) =>

2758 + prevProps.prop === nextProps.prop;

2759 + const MemoizedChild = React.memo(Child, isEqual);

2760 +

2761 + act(() => {

2762 + ReactNoop.render(

2763 + <Wrapper>

2764 + <MemoizedChild key={1} />

2765 + </Wrapper>,

2766 + );

2767 + });

2768 + expect(Scheduler).toHaveYielded([

2769 + 'render',

2770 + 'layout create',

2771 + 'passive create',

2772 + ]);

2773 +

2774 + // Include at least one no-op (memoized) update to trigger original bug.

2775 + act(() => {

2776 + ReactNoop.render(

2777 + <Wrapper>

2778 + <MemoizedChild key={1} />

2779 + </Wrapper>,

2780 + );

2781 + });

2782 + expect(Scheduler).toHaveYielded([]);

2783 +

2784 + act(() => {

2785 + ReactNoop.render(

2786 + <Wrapper>

2787 + <MemoizedChild key={2} />

2788 + </Wrapper>,

2789 + );

2790 + });

2791 + expect(Scheduler).toHaveYielded([

2792 + 'render',

2793 + 'layout destroy',

2794 + 'layout create',

2795 + 'passive destroy',

2796 + 'passive create',

2797 + ]);

2798 +

2799 + act(() => {

2800 + ReactNoop.render(null);

2801 + });

2802 + expect(Scheduler).toHaveYielded(['layout destroy', 'passive destroy']);

2803 + });

2804 +

2805 + it('calls passive effect destroy functions for descendants of memoized components', () => {

2806 + const Wrapper = ({children}) => children;

2807 + function Child() {

2808 + return <Grandchild />;

2809 + }

2810 +

2811 + function Grandchild() {

2812 + React.useEffect(() => {

2813 + Scheduler.unstable_yieldValue('passive create');

2814 + return () => {

2815 + Scheduler.unstable_yieldValue('passive destroy');

2816 + };

2817 + }, []);

2818 + React.useLayoutEffect(() => {

2819 + Scheduler.unstable_yieldValue('layout create');

2820 + return () => {

2821 + Scheduler.unstable_yieldValue('layout destroy');

2822 + };

2823 + }, []);

2824 + Scheduler.unstable_yieldValue('render');

2825 + return null;

2826 + }

2827 +

2828 + const isEqual = (prevProps, nextProps) =>

2829 + prevProps.prop === nextProps.prop;

2830 + const MemoizedChild = React.memo(Child, isEqual);

2831 +

2832 + act(() => {

2833 + ReactNoop.render(

2834 + <Wrapper>

2835 + <MemoizedChild key={1} />

2836 + </Wrapper>,

2837 + );

2838 + });

2839 + expect(Scheduler).toHaveYielded([

2840 + 'render',

2841 + 'layout create',

2842 + 'passive create',

2843 + ]);

2844 +

2845 + // Include at least one no-op (memoized) update to trigger original bug.

2846 + act(() => {

2847 + ReactNoop.render(

2848 + <Wrapper>

2849 + <MemoizedChild key={1} />

2850 + </Wrapper>,

2851 + );

2852 + });

2853 + expect(Scheduler).toHaveYielded([]);

2854 +

2855 + act(() => {

2856 + ReactNoop.render(

2857 + <Wrapper>

2858 + <MemoizedChild key={2} />

2859 + </Wrapper>,

2860 + );

2861 + });

2862 + expect(Scheduler).toHaveYielded([

2863 + 'render',

2864 + 'layout destroy',

2865 + 'layout create',

2866 + 'passive destroy',

2867 + 'passive create',

2868 + ]);

2869 +

2870 + act(() => {

2871 + ReactNoop.render(null);

2872 + });

2873 + expect(Scheduler).toHaveYielded(['layout destroy', 'passive destroy']);

2874 + });

2875 });

2876

2877 describe('useLayoutEffect', () => {