From a2df2e9a3e26d54698d3736c443a350256cb2eb4 Mon Sep 17 00:00:00 2001 From: Asa Zernik Date: Mon, 30 Sep 2019 15:45:20 -0700 Subject: [PATCH 1/2] no-op, useField: Refactor to make useMemo easier The rule against nesting of hook calls requires us to change the structure of this code somewhat. --- src/useField.js | 112 ++++++++++++++++++++++++------------------------ 1 file changed, 56 insertions(+), 56 deletions(-) diff --git a/src/useField.js b/src/useField.js index 0892beea..8eef7d78 100644 --- a/src/useField.js +++ b/src/useField.js @@ -121,63 +121,61 @@ function useField( ] ) - const handlers = { - onBlur: React.useCallback( - (event: ?SyntheticFocusEvent<*>) => { - state.blur() - if (formatOnBlur) { - /** - * Here we must fetch the value directly from Final Form because we cannot - * trust that our `state` closure has the most recent value. This is a problem - * if-and-only-if the library consumer has called `onChange()` immediately - * before calling `onBlur()`, but before the field has had a chance to receive - * the value update from Final Form. - */ - const fieldState: any = form.getFieldState(state.name) - state.change(format(fieldState.value, state.name)) - } - }, - // eslint-disable-next-line react-hooks/exhaustive-deps - [state.name, format, formatOnBlur] - ), - onChange: React.useCallback( - (event: SyntheticInputEvent<*> | any) => { - // istanbul ignore next - if (process.env.NODE_ENV !== 'production' && event && event.target) { - const targetType = event.target.type - const unknown = - ~['checkbox', 'radio', 'select-multiple'].indexOf(targetType) && - !type - - const value: any = - targetType === 'select-multiple' ? state.value : _value - - if (unknown) { - console.error( - `You must pass \`type="${ - targetType === 'select-multiple' ? 'select' : targetType - }"\` prop to your Field(${name}) component.\n` + - `Without it we don't know how to unpack your \`value\` prop - ${ - Array.isArray(value) ? `[${value}]` : `"${value}"` - }.` - ) - } - } + const onBlur = React.useCallback( + (event: ?SyntheticFocusEvent<*>) => { + state.blur() + if (formatOnBlur) { + /** + * Here we must fetch the value directly from Final Form because we cannot + * trust that our `state` closure has the most recent value. This is a problem + * if-and-only-if the library consumer has called `onChange()` immediately + * before calling `onBlur()`, but before the field has had a chance to receive + * the value update from Final Form. + */ + const fieldState: any = form.getFieldState(state.name) + state.change(format(fieldState.value, state.name)) + } + }, + // eslint-disable-next-line react-hooks/exhaustive-deps + [state.name, format, formatOnBlur] + ) + const onChange = React.useCallback( + (event: SyntheticInputEvent<*> | any) => { + // istanbul ignore next + if (process.env.NODE_ENV !== 'production' && event && event.target) { + const targetType = event.target.type + const unknown = + ~['checkbox', 'radio', 'select-multiple'].indexOf(targetType) && + !type const value: any = - event && event.target - ? getValue(event, state.value, _value, isReactNative) - : event - state.change(parse(value, name)) - }, - // eslint-disable-next-line react-hooks/exhaustive-deps - [_value, name, parse, state.change, state.value, type] - ), - onFocus: React.useCallback((event: ?SyntheticFocusEvent<*>) => { - state.focus() - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []) - } + targetType === 'select-multiple' ? state.value : _value + + if (unknown) { + console.error( + `You must pass \`type="${ + targetType === 'select-multiple' ? 'select' : targetType + }"\` prop to your Field(${name}) component.\n` + + `Without it we don't know how to unpack your \`value\` prop - ${ + Array.isArray(value) ? `[${value}]` : `"${value}"` + }.` + ) + } + } + + const value: any = + event && event.target + ? getValue(event, state.value, _value, isReactNative) + : event + state.change(parse(value, name)) + }, + // eslint-disable-next-line react-hooks/exhaustive-deps + [_value, name, parse, state.change, state.value, type] + ) + const onFocus = React.useCallback((event: ?SyntheticFocusEvent<*>) => { + state.focus() + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []) const meta = {} addLazyFieldMetaState(meta, state) @@ -214,7 +212,9 @@ function useField( } return undefined }, - ...handlers + onChange, + onBlur, + onFocus, } if (multiple) { From 40060aea84f52f5f7446d306017a5818b2dd77da Mon Sep 17 00:00:00 2001 From: Asa Zernik Date: Mon, 30 Sep 2019 15:51:11 -0700 Subject: [PATCH 2/2] src/useField: Memoize members of FieldRenderProps If the child components of check for reference equality of their props to decide to re-render, the old code was causing excessive re-renders by changing the identity of input and meta props. --- src/useField.js | 106 +++++++++++++++++++++++++++++------------------- 1 file changed, 64 insertions(+), 42 deletions(-) diff --git a/src/useField.js b/src/useField.js index 8eef7d78..137b9c4d 100644 --- a/src/useField.js +++ b/src/useField.js @@ -177,52 +177,74 @@ function useField( // eslint-disable-next-line react-hooks/exhaustive-deps }, []) - const meta = {} - addLazyFieldMetaState(meta, state) - const input: FieldInputProps = { - name, - get value() { - let value = state.value - if (formatOnBlur) { - if (component === 'input') { - value = defaultFormat(value, name) - } - } else { - value = format(value, name) - } - if (value === null && !allowNull) { - value = '' - } - if (type === 'checkbox' || type === 'radio') { - return _value - } else if (component === 'select' && multiple) { - return value || [] - } - return value - }, - get checked() { - if (type === 'checkbox') { - if (_value === undefined) { - return !!state.value + const meta = React.useMemo(() => { + const lazyObj = {} + addLazyFieldMetaState(lazyObj, state) + return lazyObj + }, [state]) + + const input = React.useMemo(() => { + const memoizedInput: FieldInputProps = { + name, + get value() { + let value = state.value + if (formatOnBlur) { + if (component === 'input') { + value = defaultFormat(value, name) + } } else { - return !!(Array.isArray(state.value) && ~state.value.indexOf(_value)) + value = format(value, name) } - } else if (type === 'radio') { - return state.value === _value - } - return undefined - }, + if (value === null && !allowNull) { + value = '' + } + if (type === 'checkbox' || type === 'radio') { + return _value + } else if (component === 'select' && multiple) { + return value || [] + } + return value + }, + get checked() { + if (type === 'checkbox') { + if (_value === undefined) { + return !!state.value + } else { + return !!( + Array.isArray(state.value) && ~state.value.indexOf(_value) + ) + } + } else if (type === 'radio') { + return state.value === _value + } + return undefined + }, + onChange, + onBlur, + onFocus + } + + if (multiple) { + memoizedInput.multiple = multiple + } + if (type !== undefined) { + memoizedInput.type = type + } + return memoizedInput + }, [ + name, + type, + multiple, + component, + allowNull, + format, + formatOnBlur, + _value, + state.value, onChange, onBlur, - onFocus, - } - - if (multiple) { - input.multiple = multiple - } - if (type !== undefined) { - input.type = type - } + onFocus + ]) const renderProps: FieldRenderProps = { input, meta } // assign to force Flow check return renderProps