From 92213056d883952085b217ba4806f55dec002877 Mon Sep 17 00:00:00 2001
From: wmzy <1256573276@qq.com>
Date: Thu, 9 Feb 2023 05:48:03 +0800
Subject: [PATCH 1/3] RFC: Extends `memo` function to support mutable event
 handlers.

---
 text/0000-extends-memo-for-event-handler.md | 122 ++++++++++++++++++++
 1 file changed, 122 insertions(+)
 create mode 100644 text/0000-extends-memo-for-event-handler.md

diff --git a/text/0000-extends-memo-for-event-handler.md b/text/0000-extends-memo-for-event-handler.md
new file mode 100644
index 00000000..491d06ae
--- /dev/null
+++ b/text/0000-extends-memo-for-event-handler.md
@@ -0,0 +1,122 @@
+- Start Date: 2023-02-08
+- RFC PR: (leave this empty)
+- React Issue: (leave this empty)
+
+# Summary
+
+Extends `memo` function to support mutable event handlers.
+
+# Basic example
+
+You need not wrap any event handler by `useCallback` or `useEvent`.
+
+```js
+import OriginalSendButton from 'some-where';
+
+let SendButton = OriginalSendButton;
+
+// When you want to optimize `SendButton`, just memoization like:
+SendButton = memo(SendButton, {events: true});
+
+function Chat() {
+  const [text, setText] = useState('');
+
+  const onClick = () => {
+    sendMessage(text);
+  };
+
+  return <SendButton onClick={onClick} />;
+}
+```
+
+# Motivation
+
+Make `memo` work for mutable event handlers.
+
+It is similar with `useEvent` rfc [first motivation](https://github.com/reactjs/rfcs/blob/useevent/text/0000-useevent.md#reading-stateprops-in-event-handlers-breaks-optimizations).
+
+Before `memo` will break by mutable event handlers.
+
+So a lot of people wrap every event handler with `useCallback` or `useEvent`.
+The wrapper is annoying, and most of the time it just makes performance worse.
+
+
+# Detailed design
+
+## Internal implementation
+
+Internally, `memo` function will approximately work like this:
+
+```jsx
+// (!) Approximate behavior
+import {memo as reactMemo, useRef} from 'react';
+
+export function memo(Component, options) {
+  if (!options || typeof options === 'function') {
+    return reactMemo(Component, options);
+  }
+
+  const {events, propsAreEqual} = options;
+
+  if (options.events) {
+
+    function Wrapped(props) {
+      const memoEvents = useRef({}).current;
+      const eventNames = Array.isArray(events)
+        ? events
+        : Object.keys(props).filter((k) => /^on[A-Z]/.test(k));
+      for (const e of eventNames) {
+        memoEvents[e] = memoEvents[e] || wrapEvent(props[e]);
+      }
+      return <Component {...props} {...memoEvents} />;
+    }
+
+    return reactMemo(Wrapped);
+  }
+
+  return reactMemo(Component, propsAreEqual);
+}
+
+const ws = new WeakSet();
+
+function wrapEvent(func) {
+  if (typeof func !== 'function' || ws.has(func)) return func;
+  const wrapped = (...params) => func(...params);
+  ws.add(wrapped);
+  return wrapped;
+}
+```
+
+IMHO, the essence of the problem is that the event handler should not be regarded as an ordinary callback.
+So we need to add a proxy layer to the event as above.
+
+# Drawbacks
+
+- If the event props does not follow the naming convention, it will cause unnecessary wrapping.
+- There will be some performance overhead for detecting event props every render.
+- In order to be compatible with the original behavior, the default parameter settings may not be reasonable.
+
+# Alternatives
+
+`useCallback`, `useEvent` and some other wrapper hooks put too much mental load on the user.
+User may don not know when to use or not. This can easily lead to premature optimization and reverse optimization.
+
+
+# Adoption strategy
+
+Release it in a minor. 
+Use the linter to advise users to remove `useCallback` wrappers for functions starting with on* or handle*.
+Write new documentation teaching common patterns.
+
+Warn for `memo` without `events` option.
+
+# How we teach this
+
+In the `memo` documentation, recommend to use this feature.
+
+In the `useCallback` documentation, emphasis callback and event handler are two different concepts.
+
+# Unresolved questions
+
+- How to teach and warn people not use event handler as callback on render time?
+- How to add the similar behavior to `PureComponent` and `shouldComponentUpdate`.

From f5bdb995c8073937a69eac23cdd8e47c4c7ae182 Mon Sep 17 00:00:00 2001
From: wmzy <1256573276@qq.com>
Date: Wed, 31 May 2023 11:23:02 +0800
Subject: [PATCH 2/3] fix: internal implementation

---
 text/0000-extends-memo-for-event-handler.md | 61 +++++++++++++--------
 1 file changed, 39 insertions(+), 22 deletions(-)

diff --git a/text/0000-extends-memo-for-event-handler.md b/text/0000-extends-memo-for-event-handler.md
index 491d06ae..856c53ea 100644
--- a/text/0000-extends-memo-for-event-handler.md
+++ b/text/0000-extends-memo-for-event-handler.md
@@ -49,42 +49,59 @@ Internally, `memo` function will approximately work like this:
 
 ```jsx
 // (!) Approximate behavior
-import {memo as reactMemo, useRef} from 'react';
+import {memo as reactMemo, forwardRef, useRef} from 'react';
 
-export function memo(Component, options) {
+export default function memo(WrappedComponent, options) {
   if (!options || typeof options === 'function') {
-    return reactMemo(Component, options);
+    return reactMemo(WrappedComponent, options);
   }
 
-  const {events, propsAreEqual} = options;
-
-  if (options.events) {
-
-    function Wrapped(props) {
-      const memoEvents = useRef({}).current;
+  let Wrapped = WrappedComponent;
+  const {events} = options;
+  if (events) {
+    // events is an array or true
+    Wrapped = forwardRef((props, ref) => {
+      const memoEventsRef = useRef({});
       const eventNames = Array.isArray(events)
         ? events
         : Object.keys(props).filter((k) => /^on[A-Z]/.test(k));
-      for (const e of eventNames) {
-        memoEvents[e] = memoEvents[e] || wrapEvent(props[e]);
-      }
-      return <Component {...props} {...memoEvents} />;
-    }
 
-    return reactMemo(Wrapped);
+      const memoEvents = eventNames.reduce(
+        (me, n) => ({
+          ...me,
+          [n]: fixEventHandler(memoEventsRef.current[e], props[e])
+        }),
+        {}
+      );
+      memoEventsRef.current = memoEvents;
+      return <WrappedComponent ref={ref} {...props} {...memoEvents} />;
+    });
+
+    Wrapped.displayName = `FixedEvent(${getDisplayName(WrappedComponent)})`;
   }
 
-  return reactMemo(Component, propsAreEqual);
+  return reactMemo(Wrapped, options.propsAreEqual);
 }
 
-const ws = new WeakSet();
+const wm = new WeakMap();
+
+function fixEventHandler(fixed, handler) {
+  if (typeof handler !== 'function' || wm.has(handler)) return handler;
+  if (!fixed) {
+    fixed = (...params) => {
+      const h = wm.get(fixed);
+      return h(...params);
+    };
+  }
 
-function wrapEvent(func) {
-  if (typeof func !== 'function' || ws.has(func)) return func;
-  const wrapped = (...params) => func(...params);
-  ws.add(wrapped);
-  return wrapped;
+  wm.set(fixed, handler);
+  return fixed;
 }
+
+function getDisplayName(WrappedComponent) {
+  return WrappedComponent.displayName || WrappedComponent.name || 'Component';
+}
+
 ```
 
 IMHO, the essence of the problem is that the event handler should not be regarded as an ordinary callback.

From a36e28f71239b4aebcc1d705aa2a86cdf98d3f93 Mon Sep 17 00:00:00 2001
From: wmzy <1256573276@qq.com>
Date: Wed, 31 May 2023 20:32:42 +0800
Subject: [PATCH 3/3] fix: memo order

---
 text/0000-extends-memo-for-event-handler.md | 48 +++++++++++----------
 1 file changed, 25 insertions(+), 23 deletions(-)

diff --git a/text/0000-extends-memo-for-event-handler.md b/text/0000-extends-memo-for-event-handler.md
index 856c53ea..150d1209 100644
--- a/text/0000-extends-memo-for-event-handler.md
+++ b/text/0000-extends-memo-for-event-handler.md
@@ -56,31 +56,33 @@ export default function memo(WrappedComponent, options) {
     return reactMemo(WrappedComponent, options);
   }
 
-  let Wrapped = WrappedComponent;
+  const MemoComponent = reactMemo(Wrapped, options.propsAreEqual);
+
   const {events} = options;
-  if (events) {
-    // events is an array or true
-    Wrapped = forwardRef((props, ref) => {
-      const memoEventsRef = useRef({});
-      const eventNames = Array.isArray(events)
-        ? events
-        : Object.keys(props).filter((k) => /^on[A-Z]/.test(k));
-
-      const memoEvents = eventNames.reduce(
-        (me, n) => ({
-          ...me,
-          [n]: fixEventHandler(memoEventsRef.current[e], props[e])
-        }),
-        {}
-      );
-      memoEventsRef.current = memoEvents;
-      return <WrappedComponent ref={ref} {...props} {...memoEvents} />;
-    });
-
-    Wrapped.displayName = `FixedEvent(${getDisplayName(WrappedComponent)})`;
-  }
 
-  return reactMemo(Wrapped, options.propsAreEqual);
+  if (!events) return MemoComponent;
+
+  // events is an array or true
+  const Wrapped = forwardRef((props, ref) => {
+    const memoEventsRef = useRef({});
+    const eventNames = Array.isArray(events)
+      ? events
+      : Object.keys(props).filter((k) => /^on[A-Z]/.test(k));
+
+    const memoEvents = eventNames.reduce(
+      (me, n) => ({
+        ...me,
+        [n]: fixEventHandler(memoEventsRef.current[e], props[e])
+      }),
+      {}
+    );
+    memoEventsRef.current = memoEvents;
+    return <MemoComponent ref={ref} {...props} {...memoEvents} />;
+  });
+
+  Wrapped.displayName = `FixedEvent(${getDisplayName(MemoComponent)})`;
+
+  return Wrapped;
 }
 
 const wm = new WeakMap();