Address feedback (will be squashed)

facebook · Sep 13, 2022 · f510cc4 · f510cc4
1 parent bb85612
commit f510cc4
Show file tree

Hide file tree

Showing 6 changed files with 22 additions and 28 deletions.
diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js
@@ -48,6 +48,7 @@ import {
   enableUpdaterTracking,
   enableCache,
   enableTransitionTracing,
+  enableUseEventHook,
 } from 'shared/ReactFeatureFlags';
 import {
   FunctionComponent,
@@ -412,16 +413,10 @@ function commitBeforeMutationEffectsOnFiber(finishedWork: Fiber) {
 
   switch (finishedWork.tag) {
     case FunctionComponent: {
-      if ((flags & Update) !== NoFlags) {
-        try {
-          commitHookEffectListUnmount(
-            HookSnapshot | HookHasEffect,
-            finishedWork,
-            finishedWork.return,
-          );
+      if (enableUseEventHook) {
+        if ((flags & Update) !== NoFlags) {
+          // useEvent doesn't need to be cleaned up
           commitHookEffectListMount(HookSnapshot | HookHasEffect, finishedWork);
-        } catch (error) {
-          captureCommitPhaseError(finishedWork, finishedWork.return, error);
         }
       }
       break;

diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js
@@ -48,6 +48,7 @@ import {
   enableUpdaterTracking,
   enableCache,
   enableTransitionTracing,
+  enableUseEventHook,
 } from 'shared/ReactFeatureFlags';
 import {
   FunctionComponent,
@@ -412,16 +413,10 @@ function commitBeforeMutationEffectsOnFiber(finishedWork: Fiber) {
 
   switch (finishedWork.tag) {
     case FunctionComponent: {
-      if ((flags & Update) !== NoFlags) {
-        try {
-          commitHookEffectListUnmount(
-            HookSnapshot | HookHasEffect,
-            finishedWork,
-            finishedWork.return,
-          );
+      if (enableUseEventHook) {
+        if ((flags & Update) !== NoFlags) {
+          // useEvent doesn't need to be cleaned up
           commitHookEffectListMount(HookSnapshot | HookHasEffect, finishedWork);
-        } catch (error) {
-          captureCommitPhaseError(finishedWork, finishedWork.return, error);
         }
       }
       break;

diff --git a/packages/react-reconciler/src/ReactFiberHooks.new.js b/packages/react-reconciler/src/ReactFiberHooks.new.js
@@ -90,7 +90,7 @@ import {
   requestUpdateLane,
   requestEventTime,
   markSkippedUpdateLanes,
-  isAlreadyRenderingProd,
+  isInvalidExecutionContextForEventFunction,
 } from './ReactFiberWorkLoop.new';
 
 import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber';
@@ -1803,13 +1803,15 @@ function mountEvent<T>(callback: () => T): () => T {
   const hook = mountWorkInProgressHook();
   const ref = {current: callback};
 
-  function event(...args) {
-    if (isAlreadyRenderingProd()) {
+  function event() {
+    if (isInvalidExecutionContextForEventFunction()) {
       throw new Error('An event from useEvent was called during render.');
     }
-    return ref.current.apply(this, args);
+    return ref.current.apply(this, arguments);
   }
 
+  // TODO: We don't need all the overhead of an effect object since there are no deps and no
+  // clean up functions.
   mountEffectImpl(
     UpdateEffect,
     HookSnapshot,

diff --git a/packages/react-reconciler/src/ReactFiberHooks.old.js b/packages/react-reconciler/src/ReactFiberHooks.old.js
@@ -90,7 +90,7 @@ import {
   requestUpdateLane,
   requestEventTime,
   markSkippedUpdateLanes,
-  isAlreadyRenderingProd,
+  isInvalidExecutionContextForEventFunction,
 } from './ReactFiberWorkLoop.old';
 
 import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber';
@@ -1803,13 +1803,15 @@ function mountEvent<T>(callback: () => T): () => T {
   const hook = mountWorkInProgressHook();
   const ref = {current: callback};
 
-  function event(...args) {
-    if (isAlreadyRenderingProd()) {
+  function event() {
+    if (isInvalidExecutionContextForEventFunction()) {
       throw new Error('An event from useEvent was called during render.');
     }
-    return ref.current.apply(this, args);
+    return ref.current.apply(this, arguments);
   }
 
+  // TODO: We don't need all the overhead of an effect object since there are no deps and no
+  // clean up functions.
   mountEffectImpl(
     UpdateEffect,
     HookSnapshot,

diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
@@ -1603,7 +1603,7 @@ export function isAlreadyRendering() {
   );
 }
 
-export function isAlreadyRenderingProd() {
+export function isInvalidExecutionContextForEventFunction() {
   // Used to throw if certain APIs are called from the wrong context.
   return (executionContext & RenderContext) !== NoContext;
 }

diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
@@ -1603,7 +1603,7 @@ export function isAlreadyRendering() {
   );
 }
 
-export function isAlreadyRenderingProd() {
+export function isInvalidExecutionContextForEventFunction() {
   // Used to throw if certain APIs are called from the wrong context.
   return (executionContext & RenderContext) !== NoContext;
 }