@samitouri / QOS-React-2 / commits / 05f5192e81

[Partial Hydration] Dispatching events should not work until hydration commits (#16532)

* Refactor a bit to use less property access * Add test for invoking an event before mount * Add Hydration effect tag This is equivalent to a "Placement" effect in that it's a new insertion to the tree but it doesn't need an actual mutation. It is only used to determine if a subtree has actually mounted yet. * Use the Hydration flag for Roots Previous roots had a Placement flag on them as a hack for this case but since we have a special flag for it now, we can just use that. * Add Flare test

Sebastian Markbåge committed Aug 22, 2019 at 08:46 UTC 05f5192e8106d006cc3189ae68c523ca123ae297
8 files changed +335 -54
packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js
+166
@@ -25,6 +25,7 @@ describe('ReactDOMServerPartialHydration', () => {
25 ReactFeatureFlags = require('shared/ReactFeatureFlags');
26 ReactFeatureFlags.enableSuspenseServerRenderer = true;
27 ReactFeatureFlags.enableSuspenseCallback = true;
28 + ReactFeatureFlags.enableFlareAPI = true;
29
30 React = require('react');
31 ReactDOM = require('react-dom');
@@ -1729,4 +1730,169 @@ describe('ReactDOMServerPartialHydration', () => {
1730 // patched up the tree, which might mean we haven't patched the className.
1731 expect(newSpan.className).toBe('hi');
1732 });
1733 +
1734 + it('does not invoke an event on a hydrated node until it commits', async () => {
1735 + let suspend = false;
1736 + let resolve;
1737 + let promise = new Promise(resolvePromise => (resolve = resolvePromise));
1738 +
1739 + function Sibling({text}) {
1740 + if (suspend) {
1741 + throw promise;
1742 + } else {
1743 + return 'Hello';
1744 + }
1745 + }
1746 +
1747 + let clicks = 0;
1748 +
1749 + function Button() {
1750 + let [clicked, setClicked] = React.useState(false);
1751 + if (clicked) {
1752 + return null;
1753 + }
1754 + return (
1755 + <a
1756 + onClick={() => {
1757 + setClicked(true);
1758 + clicks++;
1759 + }}>
1760 + Click me
1761 + </a>
1762 + );
1763 + }
1764 +
1765 + function App() {
1766 + return (
1767 + <div>
1768 + <Suspense fallback="Loading...">
1769 + <Button />
1770 + <Sibling />
1771 + </Suspense>
1772 + </div>
1773 + );
1774 + }
1775 +
1776 + suspend = false;
1777 + let finalHTML = ReactDOMServer.renderToString(<App />);
1778 + let container = document.createElement('div');
1779 + container.innerHTML = finalHTML;
1780 +
1781 + // We need this to be in the document since we'll dispatch events on it.
1782 + document.body.appendChild(container);
1783 +
1784 + let a = container.getElementsByTagName('a')[0];
1785 +
1786 + // On the client we don't have all data yet but we want to start
1787 + // hydrating anyway.
1788 + suspend = true;
1789 + let root = ReactDOM.unstable_createRoot(container, {hydrate: true});
1790 + root.render(<App />);
1791 + Scheduler.unstable_flushAll();
1792 + jest.runAllTimers();
1793 +
1794 + expect(container.textContent).toBe('Click meHello');
1795 +
1796 + // We're now partially hydrated.
1797 + a.click();
1798 + expect(clicks).toBe(0);
1799 +
1800 + // Resolving the promise so that rendering can complete.
1801 + suspend = false;
1802 + resolve();
1803 + await promise;
1804 +
1805 + Scheduler.unstable_flushAll();
1806 + jest.runAllTimers();
1807 +
1808 + // TODO: With selective hydration the event should've been replayed
1809 + // but for now we'll have to issue it again.
1810 + act(() => {
1811 + a.click();
1812 + });
1813 +
1814 + expect(clicks).toBe(1);
1815 +
1816 + expect(container.textContent).toBe('Hello');
1817 +
1818 + document.body.removeChild(container);
1819 + });
1820 +
1821 + it('does not invoke an event on a hydrated EventResponder until it commits', async () => {
1822 + let suspend = false;
1823 + let resolve;
1824 + let promise = new Promise(resolvePromise => (resolve = resolvePromise));
1825 +
1826 + function Sibling({text}) {
1827 + if (suspend) {
1828 + throw promise;
1829 + } else {
1830 + return 'Hello';
1831 + }
1832 + }
1833 +
1834 + const onEvent = jest.fn();
1835 + const TestResponder = React.unstable_createResponder('TestEventResponder', {
1836 + targetEventTypes: ['click'],
1837 + onEvent,
1838 + });
1839 +
1840 + function Button() {
1841 + let listener = React.unstable_useResponder(TestResponder, {});
1842 + return <a listeners={listener}>Click me</a>;
1843 + }
1844 +
1845 + function App() {
1846 + return (
1847 + <div>
1848 + <Suspense fallback="Loading...">
1849 + <Button />
1850 + <Sibling />
1851 + </Suspense>
1852 + </div>
1853 + );
1854 + }
1855 +
1856 + suspend = false;
1857 + let finalHTML = ReactDOMServer.renderToString(<App />);
1858 + let container = document.createElement('div');
1859 + container.innerHTML = finalHTML;
1860 +
1861 + // We need this to be in the document since we'll dispatch events on it.
1862 + document.body.appendChild(container);
1863 +
1864 + let a = container.getElementsByTagName('a')[0];
1865 +
1866 + // On the client we don't have all data yet but we want to start
1867 + // hydrating anyway.
1868 + suspend = true;
1869 + let root = ReactDOM.unstable_createRoot(container, {hydrate: true});
1870 + root.render(<App />);
1871 + Scheduler.unstable_flushAll();
1872 + jest.runAllTimers();
1873 +
1874 + // We're now partially hydrated.
1875 + a.click();
1876 + // We should not have invoked the event yet because we're not
1877 + // yet hydrated.
1878 + expect(onEvent).toHaveBeenCalledTimes(0);
1879 +
1880 + // Resolving the promise so that rendering can complete.
1881 + suspend = false;
1882 + resolve();
1883 + await promise;
1884 +
1885 + Scheduler.unstable_flushAll();
1886 + jest.runAllTimers();
1887 +
1888 + // TODO: With selective hydration the event should've been replayed
1889 + // but for now we'll have to issue it again.
1890 + act(() => {
1891 + a.click();
1892 + });
1893 +
1894 + expect(onEvent).toHaveBeenCalledTimes(1);
1895 +
1896 + document.body.removeChild(container);
1897 + });
1898 });
packages/react-dom/src/__tests__/ReactServerRenderingHydration-test.js
+87
@@ -13,6 +13,7 @@ let React;
13 let ReactDOM;
14 let ReactDOMServer;
15 let Scheduler;
16 +let act;
17
18 // These tests rely both on ReactDOMServer and ReactDOM.
19 // If a test only needs ReactDOMServer, put it in ReactServerRendering-test instead.
@@ -23,6 +24,7 @@ describe('ReactDOMServerHydration', () => {
24 ReactDOM = require('react-dom');
25 ReactDOMServer = require('react-dom/server');
26 Scheduler = require('scheduler');
27 + act = require('react-dom/test-utils').act;
28 });
29
30 it('should have the correct mounting behavior (old hydrate API)', () => {
@@ -499,4 +501,89 @@ describe('ReactDOMServerHydration', () => {
501 Scheduler.unstable_flushAll();
502 expect(element.textContent).toBe('Hello world');
503 });
504 +
505 + it('does not invoke an event on a concurrent hydrating node until it commits', () => {
506 + function Sibling({text}) {
507 + Scheduler.unstable_yieldValue('Sibling');
508 + return <span>Sibling</span>;
509 + }
510 +
511 + function Sibling2({text}) {
512 + Scheduler.unstable_yieldValue('Sibling2');
513 + return null;
514 + }
515 +
516 + let clicks = 0;
517 +
518 + function Button() {
519 + Scheduler.unstable_yieldValue('Button');
520 + let [clicked, setClicked] = React.useState(false);
521 + if (clicked) {
522 + return null;
523 + }
524 + return (
525 + <a
526 + onClick={() => {
527 + setClicked(true);
528 + clicks++;
529 + }}>
530 + Click me
531 + </a>
532 + );
533 + }
534 +
535 + function App() {
536 + return (
537 + <div>
538 + <Button />
539 + <Sibling />
540 + <Sibling2 />
541 + </div>
542 + );
543 + }
544 +
545 + let finalHTML = ReactDOMServer.renderToString(<App />);
546 + let container = document.createElement('div');
547 + container.innerHTML = finalHTML;
548 + expect(Scheduler).toHaveYielded(['Button', 'Sibling', 'Sibling2']);
549 +
550 + // We need this to be in the document since we'll dispatch events on it.
551 + document.body.appendChild(container);
552 +
553 + let a = container.getElementsByTagName('a')[0];
554 +
555 + // Hydrate asynchronously.
556 + let root = ReactDOM.unstable_createRoot(container, {hydrate: true});
557 + root.render(<App />);
558 + // Flush part way through the render.
559 + if (__DEV__) {
560 + // In DEV effects gets double invoked.
561 + expect(Scheduler).toFlushAndYieldThrough(['Button', 'Button', 'Sibling']);
562 + } else {
563 + expect(Scheduler).toFlushAndYieldThrough(['Button', 'Sibling']);
564 + }
565 +
566 + expect(container.textContent).toBe('Click meSibling');
567 +
568 + // We're now partially hydrated.
569 + a.click();
570 + // Clicking should not invoke the event yet because we haven't committed
571 + // the hydration yet.
572 + expect(clicks).toBe(0);
573 +
574 + // Finish the rest of the hydration.
575 + expect(Scheduler).toFlushAndYield(['Sibling2']);
576 +
577 + // TODO: With selective hydration the event should've been replayed
578 + // but for now we'll have to issue it again.
579 + act(() => {
580 + a.click();
581 + });
582 +
583 + expect(clicks).toBe(1);
584 +
585 + expect(container.textContent).toBe('Sibling');
586 +
587 + document.body.removeChild(container);
588 + });
589 });
packages/react-dom/src/client/ReactDOMComponentTree.js
+14 -11
@@ -23,26 +23,29 @@ export function precacheFiberNode(hostInst, node) {
23 * ReactDOMTextComponent instance ancestor.
24 */
25 export function getClosestInstanceFromNode(node) {
26 - if (node[internalInstanceKey]) {
27 - return node[internalInstanceKey];
26 + let inst = node[internalInstanceKey];
27 + if (inst) {
28 + return inst;
29 }
30
30 - while (!node[internalInstanceKey]) {
31 - if (node.parentNode) {
32 - node = node.parentNode;
31 + do {
32 + node = node.parentNode;
33 + if (node) {
34 + inst = node[internalInstanceKey];
35 } else {
36 // Top of the tree. This node must not be part of a React tree (or is
37 // unmounted, potentially).
38 return null;
39 }
38 - }
40 + } while (!inst);
41
40 - let inst = node[internalInstanceKey];
41 - if (inst.tag === HostComponent || inst.tag === HostText) {
42 - // In Fiber, this will always be the deepest root.
43 - return inst;
42 + let tag = inst.tag;
43 + switch (tag) {
44 + case HostComponent:
45 + case HostText:
46 + // In Fiber, this will always be the deepest root.
47 + return inst;
48 }
45 -
49 return null;
50 }
51
packages/react-reconciler/src/ReactFiberBeginWork.js
+18 -6
@@ -46,6 +46,7 @@ import {
46 NoEffect,
47 PerformedWork,
48 Placement,
49 + Hydrating,
50 ContentReset,
51 DidCapture,
52 Update,
@@ -944,11 +945,10 @@ function updateHostRoot(current, workInProgress, renderExpirationTime) {
945 // be any children to hydrate which is effectively the same thing as
946 // not hydrating.
947
947 - // This is a bit of a hack. We track the host root as a placement to
948 - // know that we're currently in a mounting state. That way isMounted
949 - // works as expected. We must reset this before committing.
950 - // TODO: Delete this when we delete isMounted and findDOMNode.
951 - workInProgress.effectTag |= Placement;
948 + // Mark the host root with a Hydrating effect to know that we're
949 + // currently in a mounting state. That way isMounted, findDOMNode and
950 + // event replaying works as expected.
951 + workInProgress.effectTag |= Hydrating;
952
953 // Ensure that children mount into this root without tracking
954 // side-effects. This ensures that we don't store Placement effects on
@@ -2095,12 +2095,24 @@ function updateDehydratedSuspenseComponent(
2095 );
2096 const nextProps = workInProgress.pendingProps;
2097 const nextChildren = nextProps.children;
2098 - workInProgress.child = mountChildFibers(
2098 + const child = mountChildFibers(
2099 workInProgress,
2100 null,
2101 nextChildren,
2102 renderExpirationTime,
2103 );
2104 + let node = child;
2105 + while (node) {
2106 + // Mark each child as hydrating. This is a fast path to know whether this
2107 + // tree is part of a hydrating tree. This is used to determine if a child
2108 + // node has fully mounted yet, and for scheduling event replaying.
2109 + // Conceptually this is similar to Placement in that a new subtree is
2110 + // inserted into the React tree here. It just happens to not need DOM
2111 + // mutations because it already exists.
2112 + node.effectTag |= Hydrating;
2113 + node = node.sibling;
2114 + }
2115 + workInProgress.child = child;
2116 return workInProgress.child;
2117 }
2118 }
packages/react-reconciler/src/ReactFiberCompleteWork.js
+6 -11
@@ -56,7 +56,6 @@ import {
56 } from 'shared/ReactWorkTags';
57 import {NoMode, BatchedMode} from './ReactTypeOfMode';
58 import {
59 - Placement,
59 Ref,
60 Update,
61 NoEffect,
@@ -670,9 +669,6 @@ function completeWork(
669 // If we hydrated, pop so that we can delete any remaining children
670 // that weren't hydrated.
671 popHydrationState(workInProgress);
673 - // This resets the hacky state to fix isMounted before committing.
674 - // TODO: Delete this when we delete isMounted and findDOMNode.
675 - workInProgress.effectTag &= ~Placement;
672 }
673 updateHostContainer(workInProgress);
674 break;
@@ -859,14 +855,13 @@ function completeWork(
855 if ((workInProgress.effectTag & DidCapture) === NoEffect) {
856 // This boundary did not suspend so it's now hydrated and unsuspended.
857 workInProgress.memoizedState = null;
862 - if (enableSuspenseCallback) {
863 - // Notify the callback.
864 - workInProgress.effectTag |= Update;
865 - }
866 - } else {
867 - // Something suspended. Schedule an effect to attach retry listeners.
868 - workInProgress.effectTag |= Update;
858 }
859 + // If nothing suspended, we need to schedule an effect to mark this boundary
860 + // as having hydrated so events know that they're free be invoked.
861 + // It's also a signal to replay events and the suspense callback.
862 + // If something suspended, schedule an effect to attach retry listeners.
863 + // So we might as well always mark this.
864 + workInProgress.effectTag |= Update;
865 return null;
866 }
867 }
packages/react-reconciler/src/ReactFiberTreeReflection.js
+10 -9
@@ -23,7 +23,7 @@ import {
23 HostText,
24 FundamentalComponent,
25 } from 'shared/ReactWorkTags';
26 -import {NoEffect, Placement} from 'shared/ReactSideEffectTags';
26 +import {NoEffect, Placement, Hydrating} from 'shared/ReactSideEffectTags';
27 import {enableFundamentalAPI} from 'shared/ReactFeatureFlags';
28
29 const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner;
@@ -32,20 +32,21 @@ const MOUNTING = 1;
32 const MOUNTED = 2;
33 const UNMOUNTED = 3;
34
35 -function isFiberMountedImpl(fiber: Fiber): number {
35 +type MountState = 1 | 2 | 3;
36 +
37 +function isFiberMountedImpl(fiber: Fiber): MountState {
38 let node = fiber;
39 if (!fiber.alternate) {
40 // If there is no alternate, this might be a new tree that isn't inserted
41 // yet. If it is, then it will have a pending insertion effect on it.
40 - if ((node.effectTag & Placement) !== NoEffect) {
41 - return MOUNTING;
42 - }
43 - while (node.return) {
44 - node = node.return;
45 - if ((node.effectTag & Placement) !== NoEffect) {
42 + let nextNode = node;
43 + do {
44 + node = nextNode;
45 + if ((node.effectTag & (Placement | Hydrating)) !== NoEffect) {
46 return MOUNTING;
47 }
48 - }
48 + nextNode = node.return;
49 + } while (nextNode);
50 } else {
51 while (node.return) {
52 node = node.return;
packages/react-reconciler/src/ReactFiberWorkLoop.js
+16 -1
@@ -96,6 +96,8 @@ import {
96 Passive,
97 Incomplete,
98 HostEffectMask,
99 + Hydrating,
100 + HydratingAndUpdate,
101 } from 'shared/ReactSideEffectTags';
102 import {
103 NoWork,
@@ -1860,7 +1862,8 @@ function commitMutationEffects(root: FiberRoot, renderPriorityLevel) {
1862 // updates, and deletions. To avoid needing to add a case for every possible
1863 // bitmap value, we remove the secondary effects from the effect tag and
1864 // switch on that value.
1863 - let primaryEffectTag = effectTag & (Placement | Update | Deletion);
1865 + let primaryEffectTag =
1866 + effectTag & (Placement | Update | Deletion | Hydrating);
1867 switch (primaryEffectTag) {
1868 case Placement: {
1869 commitPlacement(nextEffect);
@@ -1883,6 +1886,18 @@ function commitMutationEffects(root: FiberRoot, renderPriorityLevel) {
1886 commitWork(current, nextEffect);
1887 break;
1888 }
1889 + case Hydrating: {
1890 + nextEffect.effectTag &= ~Hydrating;
1891 + break;
1892 + }
1893 + case HydratingAndUpdate: {
1894 + nextEffect.effectTag &= ~Hydrating;
1895 +
1896 + // Update
1897 + const current = nextEffect.alternate;
1898 + commitWork(current, nextEffect);
1899 + break;
1900 + }
1901 case Update: {
1902 const current = nextEffect.alternate;
1903 commitWork(current, nextEffect);
packages/shared/ReactSideEffectTags.js
+18 -16
@@ -10,26 +10,28 @@
10 export type SideEffectTag = number;
11
12 // Don't change these two values. They're used by React Dev Tools.
13 -export const NoEffect = /* */ 0b000000000000;
14 -export const PerformedWork = /* */ 0b000000000001;
13 +export const NoEffect = /* */ 0b0000000000000;
14 +export const PerformedWork = /* */ 0b0000000000001;
15
16 // You can change the rest (and add more).
17 -export const Placement = /* */ 0b000000000010;
18 -export const Update = /* */ 0b000000000100;
19 -export const PlacementAndUpdate = /* */ 0b000000000110;
20 -export const Deletion = /* */ 0b000000001000;
21 -export const ContentReset = /* */ 0b000000010000;
22 -export const Callback = /* */ 0b000000100000;
23 -export const DidCapture = /* */ 0b000001000000;
24 -export const Ref = /* */ 0b000010000000;
25 -export const Snapshot = /* */ 0b000100000000;
26 -export const Passive = /* */ 0b001000000000;
17 +export const Placement = /* */ 0b0000000000010;
18 +export const Update = /* */ 0b0000000000100;
19 +export const PlacementAndUpdate = /* */ 0b0000000000110;
20 +export const Deletion = /* */ 0b0000000001000;
21 +export const ContentReset = /* */ 0b0000000010000;
22 +export const Callback = /* */ 0b0000000100000;
23 +export const DidCapture = /* */ 0b0000001000000;
24 +export const Ref = /* */ 0b0000010000000;
25 +export const Snapshot = /* */ 0b0000100000000;
26 +export const Passive = /* */ 0b0001000000000;
27 +export const Hydrating = /* */ 0b0010000000000;
28 +export const HydratingAndUpdate = /* */ 0b0010000000100;
29
30 // Passive & Update & Callback & Ref & Snapshot
29 -export const LifecycleEffectMask = /* */ 0b001110100100;
31 +export const LifecycleEffectMask = /* */ 0b0001110100100;
32
33 // Union of all host effects
32 -export const HostEffectMask = /* */ 0b001111111111;
34 +export const HostEffectMask = /* */ 0b0011111111111;
35
34 -export const Incomplete = /* */ 0b010000000000;
35 -export const ShouldCapture = /* */ 0b100000000000;
36 +export const Incomplete = /* */ 0b0100000000000;
37 +export const ShouldCapture = /* */ 0b1000000000000;