Prevent stale closures of memoized callbacks (#137)

* improve callback memoization

* add useWarnOnce hook

Author: wsmd

src/useCache.js

@@ -1,7 +1,7 @@
 import { useRef } from 'react';
 import { parseInputArgs } from './parseInputArgs';
 import { useInputId } from './useInputId';
-import { useCache } from './useCache';
+import { useMap, useReferencedCallback, useWarnOnce } from './utils-hooks';
 import { useState } from './useState';
 import {
   noop,
@@ -36,16 +36,9 @@ export default function useFormState(initialState, options) {
 
   const formState = useState({ initialState });
   const { getIdProp } = useInputId(formOptions.withIds);
-  const { set: setDirty, get: isDirty } = useCache();
-  const callbacks = useCache();
-  const devWarnings = useCache();
-
-  function warn(key, type, message) {
-    if (!devWarnings.has(`${type}:${key}`)) {
-      devWarnings.set(`${type}:${key}`, true);
-      console.warn('[useFormState]', message);
-    }
-  }
+  const { set: setDirty, get: isDirty } = useMap();
+  const referencedCallback = useReferencedCallback();
+  const warn = useWarnOnce();
 
   const createPropsGetter = type => (...args) => {
     const inputOptions = parseInputArgs(args);
@@ -69,8 +62,7 @@ export default function useFormState(initialState, options) {
       if (__DEV__) {
         if (isRaw) {
           warn(
-            key,
-            'missingInitialValue',
+            `missingInitialValue.${key}`,
             `The initial value for input "${name}" is missing. Custom inputs ` +
               'controlled with raw() are expected to have an initial value ' +
               'provided to useFormState(). To prevent React from treating ' +
@@ -124,8 +116,7 @@ export default function useFormState(initialState, options) {
         if (__DEV__) {
           if (isRaw && ![value, other].every(testIsEqualCompatibility)) {
             warn(
-              key,
-              'missingCompare',
+              `missingCompare.${key}`,
               `You used a raw input type for "${name}" without providing a ` +
                 'custom compare method. As a result, the pristine value of ' +
                 'this input will be calculated using strict equality check ' +
@@ -164,8 +155,7 @@ export default function useFormState(initialState, options) {
         error = e.target.validationMessage;
       } else if (__DEV__) {
         warn(
-          key,
-          'missingValidate',
+          `missingValidate.${key}`,
           `You used a raw input type for "${name}" without providing a ` +
             'custom validate method. As a result, validation of this input ' +
             'will be set to "true" automatically. If you need to validate ' +
@@ -243,7 +233,7 @@ export default function useFormState(initialState, options) {
 
         return hasValueInState ? formState.current.values[name] : '';
       },
-      onChange: callbacks.getOrSet(`onChange.${key}`, e => {
+      onChange: referencedCallback(`onChange.${key}`, e => {
         setDirty(name, true);
         let value;
         if (isRaw) {
@@ -256,8 +246,7 @@ export default function useFormState(initialState, options) {
             /* istanbul ignore else */
             if (__DEV__) {
               warn(
-                key,
-                'onChangeUndefined',
+                `onChangeUndefined.${key}`,
                 `You used a raw input type for "${name}" with an onChange() ` +
                   'option without returning a value. The onChange callback ' +
                   'of raw inputs, when provided, is used to determine the ' +
@@ -296,7 +285,7 @@ export default function useFormState(initialState, options) {
 
         formState.setValues(partialNewState);
       }),
-      onBlur: callbacks.getOrSet(`onBlur.${key}`, e => {
+      onBlur: referencedCallback(`onBlur.${key}`, e => {
         touch(e);
 
         inputOptions.onBlur(e);

src/useFormState.js

@@ -1,15 +1,15 @@
 import { useReducer, useRef } from 'react';
 import { isFunction, isEqual } from './utils';
-import { useCache } from './useCache';
+import { useMap } from './utils-hooks';
 
 function stateReducer(state, newState) {
   return isFunction(newState) ? newState(state) : { ...state, ...newState };
 }
 
 export function useState({ initialState }) {
   const state = useRef();
-  const initialValues = useCache();
-  const comparators = useCache();
+  const initialValues = useMap();
+  const comparators = useMap();
   const [values, setValues] = useReducer(stateReducer, initialState || {});
   const [touched, setTouched] = useReducer(stateReducer, {});
   const [validity, setValidity] = useReducer(stateReducer, {});

src/useState.js

@@ -0,0 +1,32 @@
+import { useRef } from 'react';
+
+export function useMap() {
+  const map = useRef(new Map());
+  return {
+    set: (key, value) => map.current.set(key, value),
+    has: key => map.current.has(key),
+    get: key => map.current.get(key),
+  };
+}
+
+export function useReferencedCallback() {
+  const callbacks = useMap();
+  return (key, current) => {
+    if (!callbacks.has(key)) {
+      const callback = (...args) => callback.current(...args);
+      callbacks.set(key, callback);
+    }
+    callbacks.get(key).current = current;
+    return callbacks.get(key);
+  };
+}
+
+export function useWarnOnce() {
+  const didWarnRef = useRef(new Set());
+  return (key, message) => {
+    if (!didWarnRef.current.has(key)) {
+      didWarnRef.current.add(key);
+      console.warn('[useFormState]', message);
+    }
+  };
+}

src/utils-hooks.js

@@ -4,6 +4,8 @@ import { useFormState } from '../src';
 
 export { renderHook } from 'react-hooks-testing-library';
 
+export { render as renderElement, fireEvent };
+
 export const InputTypes = {
   textLike: ['text', 'email', 'password', 'search', 'tel', 'url'],
   time: ['date', 'month', 'time', 'week'],

test/test-utils.js

@@ -1,6 +1,12 @@
-import React from 'react';
+import React, { useState } from 'react';
 import { useFormState } from '../src';
-import { renderWithFormState, renderHook, InputTypes } from './test-utils';
+import {
+  InputTypes,
+  fireEvent,
+  renderHook,
+  renderElement,
+  renderWithFormState,
+} from './test-utils';
 
 describe('input type methods return correct props object', () => {
   /**
@@ -553,4 +559,34 @@ describe('Input props are memoized', () => {
     change({ value: 'c' }, root.childNodes[1]);
     expect(renderCheck).toHaveBeenCalledTimes(2);
   });
+
+  it('prevents callbacks from using stale closures', () => {
+    const onInputChange = jest.fn();
+
+    function ComponentWithInternalState() {
+      const [state, setState] = useState(1);
+      const [, { text }] = useFormState(null, {
+        onBlur: () => {
+          onInputChange(state);
+          setState(state + 1);
+        },
+        onChange: () => {
+          onInputChange(state);
+          setState(state + 1);
+        },
+      });
+      return <input {...text('name')} />;
+    }
+
+    const { container } = renderElement(<ComponentWithInternalState />);
+
+    fireEvent.change(container.firstChild, { target: { value: 'foo' } });
+    expect(onInputChange).toHaveBeenLastCalledWith(1);
+
+    fireEvent.change(container.firstChild, { target: { value: 'bar' } });
+    expect(onInputChange).toHaveBeenLastCalledWith(2);
+
+    fireEvent.blur(container.firstChild);
+    expect(onInputChange).toHaveBeenLastCalledWith(3);
+  });
 });