Skip to content

Commit d962dd9

Browse files
derrickbeiningwsmd
authored andcommitted
Fix stale closures of event handlers (#76)
* Fix stale closures of callback options in inputs.text et. al. The implementation of useFormState is problematic because it caches callback functions passed to inputs.text et. al. This causes functions like onChange, validate, and onBlur to run with stale closures, namely, the closure with which they are initialised. This commit fixes the issue by removing the `callback` cache from the implementation. Tests that depended on this buggy behavior were removed as they are unsupportable features given the current implementation. * Re-add getOrSet to useCache
1 parent e97049a commit d962dd9

File tree

3 files changed

+4
-30
lines changed

3 files changed

+4
-30
lines changed

src/constants.js

-3
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,4 @@ export const TYPES = [
4646
WEEK,
4747
];
4848

49-
export const ON_CHANGE_HANDLER = 0;
50-
export const ON_BLUR_HANDLER = 1;
51-
5249
export const CONSOLE_TAG = '[useFormState]';

src/useFormState.js

+4-7
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,6 @@ import {
1212
TEXTAREA,
1313
SELECT_MULTIPLE,
1414
LABEL,
15-
ON_CHANGE_HANDLER,
16-
ON_BLUR_HANDLER,
1715
CONSOLE_TAG,
1816
} from './constants';
1917

@@ -31,7 +29,6 @@ export default function useFormState(initialState, options) {
3129
const formState = useState({ initialState, ...formOptions });
3230
const { getIdProp } = useInputId(formOptions.withIds);
3331
const { set: setDirty, has: isDirty } = useCache();
34-
const callbacks = useCache();
3532
const devWarnings = useCache();
3633

3734
function warn(key, type, message) {
@@ -202,7 +199,7 @@ export default function useFormState(initialState, options) {
202199

203200
return hasValueInState ? formState.current.values[name] : '';
204201
},
205-
onChange: callbacks.getOrSet(ON_BLUR_HANDLER + key, e => {
202+
onChange: e => {
206203
setDirty(name, true);
207204
let value;
208205
if (isRaw) {
@@ -252,8 +249,8 @@ export default function useFormState(initialState, options) {
252249
}
253250

254251
formState.setValues(partialNewState);
255-
}),
256-
onBlur: callbacks.getOrSet(ON_CHANGE_HANDLER + key, e => {
252+
},
253+
onBlur: e => {
257254
touch(e);
258255

259256
inputOptions.onBlur(e);
@@ -269,7 +266,7 @@ export default function useFormState(initialState, options) {
269266
validate(e);
270267
setDirty(name, false);
271268
}
272-
}),
269+
},
273270
...getIdProp('id', name, ownValue),
274271
};
275272

test/useFormState-input.test.js

-20
Original file line numberDiff line numberDiff line change
@@ -534,23 +534,3 @@ describe('Input blur behavior', () => {
534534
);
535535
});
536536
});
537-
538-
describe('Input props are memoized', () => {
539-
it('does not cause re-render of memoized components', () => {
540-
const renderCheck = jest.fn(() => true);
541-
const MemoInput = React.memo(
542-
props => renderCheck() && <input {...props} />,
543-
);
544-
const { change, root } = renderWithFormState(([, { text }]) => (
545-
<div>
546-
<input {...text('foo')} />
547-
<MemoInput {...text('bar')} />
548-
</div>
549-
));
550-
change({ value: 'a' }, root.childNodes[0]);
551-
change({ value: 'b' }, root.childNodes[0]);
552-
expect(renderCheck).toHaveBeenCalledTimes(1);
553-
change({ value: 'c' }, root.childNodes[1]);
554-
expect(renderCheck).toHaveBeenCalledTimes(2);
555-
});
556-
});

0 commit comments

Comments
 (0)