Move EventComponent state creation to complete phase + tests (#15352)

facebook · Apr 8, 2019 · 29fb586 · 29fb586
1 parent 745baf2
commit 29fb586
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 19 deletions.
diff --git a/packages/react-dom/src/events/DOMEventResponderSystem.js b/packages/react-dom/src/events/DOMEventResponderSystem.js
@@ -56,7 +56,6 @@ type PartialEventObject = {
 
 let currentOwner = null;
 let currentFiber: Fiber;
-let currentResponder: ReactEventResponder;
 let currentEventQueue: EventQueue;
 
 const eventResponderContext: ResponderContext = {
@@ -227,22 +226,18 @@ const eventResponderContext: ResponderContext = {
     return false;
   },
   setTimeout(func: () => void, delay): TimeoutID {
-    const contextResponder = currentResponder;
     const contextFiber = currentFiber;
     return setTimeout(() => {
       const previousEventQueue = currentEventQueue;
       const previousFiber = currentFiber;
-      const previousResponder = currentResponder;
       currentEventQueue = createEventQueue();
-      currentResponder = contextResponder;
       currentFiber = contextFiber;
       try {
         func();
         batchedUpdates(processEventQueue, currentEventQueue);
       } finally {
         currentFiber = previousFiber;
         currentEventQueue = previousEventQueue;
-        currentResponder = previousResponder;
       }
     }, delay);
   },
@@ -361,19 +356,12 @@ function handleTopLevelType(
     }
   }
   let {props, state} = fiber.stateNode;
-  if (state === null && responder.createInitialState !== undefined) {
-    state = fiber.stateNode.state = responder.createInitialState(props);
-  }
   const previousFiber = currentFiber;
-  const previousResponder = currentResponder;
   currentFiber = fiber;
-  currentResponder = responder;
-
   try {
     responder.onEvent(responderEvent, eventResponderContext, props, state);
   } finally {
     currentFiber = previousFiber;
-    currentResponder = previousResponder;
   }
 }
 
@@ -430,16 +418,13 @@ export function unmountEventResponder(
     let {props, state} = fiber.stateNode;
     const previousEventQueue = currentEventQueue;
     const previousFiber = currentFiber;
-    const previousResponder = currentResponder;
     currentEventQueue = createEventQueue();
     currentFiber = fiber;
-    currentResponder = responder;
     try {
       onUnmount(eventResponderContext, props, state);
     } finally {
       currentEventQueue = previousEventQueue;
       currentFiber = previousFiber;
-      currentResponder = previousResponder;
     }
   }
   if (currentOwner === fiber) {

diff --git a/packages/react-dom/src/events/__tests__/DOMEventResponderSystem-test.internal.js b/packages/react-dom/src/events/__tests__/DOMEventResponderSystem-test.internal.js
@@ -13,9 +13,15 @@ let React;
 let ReactFeatureFlags;
 let ReactDOM;
 
-function createReactEventComponent(targetEventTypes, onEvent, onUnmount) {
+function createReactEventComponent(
+  targetEventTypes,
+  createInitialState,
+  onEvent,
+  onUnmount,
+) {
   const testEventResponder = {
     targetEventTypes,
+    createInitialState,
     onEvent,
     onUnmount,
   };
@@ -61,6 +67,7 @@ describe('DOMEventResponderSystem', () => {
 
     const ClickEventComponent = createReactEventComponent(
       ['click'],
+      undefined,
       (event, context, props) => {
         eventResponderFiredCount++;
         eventLog.push({
@@ -114,6 +121,7 @@ describe('DOMEventResponderSystem', () => {
 
     const ClickEventComponent = createReactEventComponent(
       ['click'],
+      undefined,
       (event, context, props) => {
         eventLog.push({
           name: event.type,
@@ -149,6 +157,7 @@ describe('DOMEventResponderSystem', () => {
 
     const ClickEventComponent = createReactEventComponent(
       ['click'],
+      undefined,
       (event, context, props) => {
         eventResponderFiredCount++;
         eventLog.push({
@@ -193,13 +202,15 @@ describe('DOMEventResponderSystem', () => {
 
     const ClickEventComponentA = createReactEventComponent(
       ['click'],
+      undefined,
       (context, props) => {
         eventLog.push('A');
       },
     );
 
     const ClickEventComponentB = createReactEventComponent(
       ['click'],
+      undefined,
       (context, props) => {
         eventLog.push('B');
       },
@@ -228,6 +239,7 @@ describe('DOMEventResponderSystem', () => {
 
     const ClickEventComponent = createReactEventComponent(
       ['click'],
+      undefined,
       (event, context, props) => {
         if (props.onMagicClick) {
           const syntheticEvent = {
@@ -265,6 +277,7 @@ describe('DOMEventResponderSystem', () => {
 
     const LongPressEventComponent = createReactEventComponent(
       ['click'],
+      undefined,
       (event, context, props) => {
         const pressEvent = {
           listener: props.onPress,
@@ -323,7 +336,8 @@ describe('DOMEventResponderSystem', () => {
 
     const EventComponent = createReactEventComponent(
       [],
-      (event, context, props) => {},
+      undefined,
+      (event, context, props, state) => {},
       () => {
         onUnmountFired++;
       },
@@ -339,4 +353,29 @@ describe('DOMEventResponderSystem', () => {
     ReactDOM.render(null, container);
     expect(onUnmountFired).toEqual(1);
   });
+
+  it('the event responder onUnmount() function should fire with state', () => {
+    let counter = 0;
+
+    const EventComponent = createReactEventComponent(
+      [],
+      () => ({
+        incrementAmount: 5,
+      }),
+      (event, context, props, state) => {},
+      (context, props, state) => {
+        counter += state.incrementAmount;
+      },
+    );
+
+    const Test = () => (
+      <EventComponent>
+        <button />
+      </EventComponent>
+    );
+
+    ReactDOM.render(<Test />, container);
+    ReactDOM.render(null, container);
+    expect(counter).toEqual(5);
+  });
 });
diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js
@@ -774,10 +774,18 @@ function completeWork(
         popHostContext(workInProgress);
         const rootContainerInstance = getRootHostContainer();
         const responder = workInProgress.type.responder;
+        const stateNode = workInProgress.stateNode;
         // Update the props on the event component state node
-        workInProgress.stateNode.props = newProps;
+        stateNode.props = newProps;
         // Update the root container, so we can properly unmount events at some point
-        workInProgress.stateNode.rootInstance = rootContainerInstance;
+        stateNode.rootInstance = rootContainerInstance;
+        // Initialize event component state if createInitialState exists
+        if (
+          stateNode.state === null &&
+          responder.createInitialState !== undefined
+        ) {
+          stateNode.state = responder.createInitialState(newProps);
+        }
         handleEventComponent(responder, rootContainerInstance);
       }
       break;