ronantakizawa/codereview-bench huggingface dataset:
Repository: react-hook-form/react-hook-form
File: src/__tests__/useForm.test.tsx (TypeScript)

Code before:
        await act(async () => {
          result.current.trigger(['test.sub', 'test1']);
        });

        const fields2 = {
          test: {
            sub: {
              hasValidated: true,
              mount: true,
              name: 'test.sub',
              ref: { name: 'test.sub' },
            },
          },
          test1: {
            hasValidated: true,
            mount: true,
            name: 'test1',
            ref: {
              name: 'test1',
            },
          },
        };

        expect(resolver).toHaveBeenNthCalledWith(3, defaultValues, undefined, {
          criteriaMode: undefined,
          fields: fields2,
          names: ['test.sub', 'test1'],
        });
      });
    });
  });

  describe('updateValid', () => {
    it('should be called resolver with default values if default value is defined', async () => {
      type FormValues = {
        test: string;
      };

      const resolver = jest.fn(async (data: FormValues) => {
        return {
          values: data,
          errors: {},
        };
      });

      const { result } = renderHook(() =>
        useForm<FormValues>({
          resolver,
          defaultValues: { test: 'default' },
        }),
      );

Review comment (suggestion):
This is something I couldn't figure out with Typescript - I wanted to just add hasValidated to test1 here, but I was getting some kind of typescript error about it and I couldn't figure out how to resolve it. So I'm sure there's a better way to do this.

Code after:
          criteriaMode: undefined,
          fields,
          names: ['test.sub', 'test1'],
        });
      });
    });
  });

  describe('updateValid', () => {
    it('should be called resolver with default values if default value is defined', async () => {
      type FormValues = {
        test: string;
      };

      const resolver = jest.fn(async (data: FormValues) => {
        return {
          values: data,
          errors: {},
        };
      });

      const { result } = renderHook(() =>
        useForm<FormValues>({
          resolver,
          defaultValues: { test: 'default' },
        }),
      );

      const { ref } = result.current.register('test');

      isFunction(ref) &&
        ref({
          target: {
            value: '',
          },
        });

      await act(async () => {
        await result.current.trigger();
      });

      expect(resolver).toHaveBeenCalledWith(
        {
          test: 'default',
        },
        undefined,
        {
          criteriaMode: undefined,
          fields: {
            test: {
              mount: true,
Repository: react-hook-form/react-hook-form
File: src/__tests__/useForm/register.test.tsx (TypeScript)

Code before:
            test: '1234',
          });
        }, [reset]);

        return (
          <form>
            <input
              {...register('test', { setValueAs: (value) => value + '5' })}
            />
          </form>
        );
      };

      render(<App />);

      expect((screen.getByRole('textbox') as HTMLInputElement).value).toEqual(
        '1234',
      );
    });

    it('should populate input as string and submit as datetime object ', async () => {
      let submitData: unknown;

      const App = () => {
        const { register, handleSubmit } = useForm<{
          test: Date | string;
        }>({
          defaultValues: {
            test: '2020-10-10',
          },
        });

        return (
          <form
            onSubmit={handleSubmit((data) => {
              submitData = data;
            })}
          >
            <input {...register('test', { valueAsDate: true })} />
            <button>Submit</button>
          </form>
        );
      };

      render(<App />);

      expect((screen.getByRole('textbox') as HTMLInputElement).value).toEqual(
        '2020-10-10',
      );

      await actComponent(async () => {

Review comment (suggestion):
@bluebill1049 With this PR fix, when users use the `valueAs` option, they must now define it as a Union type.

- valueAsDate: `Date | string`
- valueAsNumber: `number | string`
- setValueAs: `xxx | string`

This is a by-product of fixing the type correctly.
I think you and I discussed earlier whether the `FormValues` type is the field values type or the submit values type. I believe it is both.

Code after:
                  return item;
                });

                setCheckboxValue(updatedValue);
                onChange(updatedValue);
              }}
              type="checkbox"
              checked={checkboxValue[index]}
            />
          ))}
        </div>
      );
    };

    function App() {
      const { control, watch } = useForm({
        defaultValues: {
          test: [true, false, false],
        },
      });
      inputs = control._fields;
      watchedValue.push(watch());

      return (
        <form>
          <Controller
            name="test"
            control={control}
            render={({ field }) => (
              <Checkboxes onChange={field.onChange} value={field.value} />
            )}
          />
          <input type="submit" />
        </form>
      );
    }

    render(<App />);

    fireEvent.click(screen.getAllByRole('checkbox')[0]);

    expect(watchedValue).toEqual([
      { test: [true, false, false] },
      { test: [false, false, false] },
    ]);

    expect(inputs).toMatchSnapshot();
  });

  describe('when setValueAs is presented with inputs', () => {
    it('should not update inputs correctly with useForm defaultValues', () => {
Repository: react-hook-form/react-hook-form
File: src/useController.ts (TypeScript)

Code before:
import * as React from 'react';

import getControllerValue from './logic/getControllerValue';
import isNameInFieldArray from './logic/isNameInFieldArray';
import get from './utils/get';
import { EVENTS } from './constants';
import {
  Field,
  FieldPathWithValue,
  FieldValues,
  InternalFieldName,
  UseControllerProps,
  UseControllerReturn,
} from './types';
import { useFormContext } from './useFormContext';
import { useFormState } from './useFormState';

export function useController<
  TFieldValues extends FieldValues = FieldValues,
  TResult = any,
  TName extends FieldPathWithValue<TFieldValues, TResult> = FieldPathWithValue<
    TFieldValues,
    TResult
  >,
>(
  props: UseControllerProps<TFieldValues, TResult, TName>,
): UseControllerReturn<TFieldValues, TResult, TName> {
  const methods = useFormContext<TFieldValues>();
  const { name, control = methods.control, shouldUnregister } = props;
  const [value, setInputStateValue] = React.useState(
    get(
      control._formValues,
      name,
      get(control._defaultValues, name, props.defaultValue),
    ),
  );
  const formState = useFormState({
    control: control || methods.control,
    name,
  });

  const registerProps = control.register(name, {
    ...props.rules,
    value,
  });


Review comment (suggestion):
This needs to be `any` if unspecified because we want `TName` to be able to be one of all the possible field paths of `TFieldValues`.
There's a check where it's actually used so `TResult` should never really be used as `any`

Code after:
import * as React from 'react';

import getControllerValue from './logic/getControllerValue';
import isNameInFieldArray from './logic/isNameInFieldArray';
import get from './utils/get';
import { EVENTS } from './constants';
import {
  Field,
  FieldPathWithValue,
  FieldValues,
  InternalFieldName,
  UseControllerProps,
  UseControllerReturn,
} from './types';
import { useFormContext } from './useFormContext';
import { useFormState } from './useFormState';
import { useSubscribe } from './useSubscribe';

export function useController<
  TFieldValues extends FieldValues = FieldValues,
  TResult = any,
  TName extends FieldPathWithValue<TFieldValues, TResult> = FieldPathWithValue<
    TFieldValues,
    TResult
  >,
>(
  props: UseControllerProps<TFieldValues, TResult, TName>,
): UseControllerReturn<TFieldValues, TResult, TName> {
  const methods = useFormContext<TFieldValues>();
  const { name, control = methods.control, shouldUnregister } = props;
  const [value, setInputStateValue] = React.useState(
    get(
      control._formValues,
      name,
      get(control._defaultValues, name, props.defaultValue),
    ),
  );
  const formState = useFormState({
    control: control || methods.control,
    name,
  });
  const _name = React.useRef(name);
  _name.current = name;

  useSubscribe({
    subject: control._subjects.control,
Repository: react-hook-form/react-hook-form
File: src/types/controller.ts (TypeScript)

Code before:
import * as React from 'react';

import { RegisterOptions } from './validator';
import {
  Control,
  DeepMapImpl,
  DeepPartialImpl,
  FieldError,
  FieldPath,
  FieldPathValue,
  FieldValues,
  RefCallBack,
  UnionLike,
  UnpackNestedValue,
  UseFormStateReturn,
} from './';

type ControllerFieldError<T> = DeepMapImpl<
  DeepPartialImpl<UnionLike<T>>,
  FieldError
>;

export type ControllerFieldState<
  TFieldValues extends FieldValues = FieldValues,
  TFieldName extends FieldPath<TFieldValues> = FieldPath<TFieldValues>,
> = {
  invalid: boolean;
  isTouched: boolean;
  isDirty: boolean;
  error?: ControllerFieldError<FieldPathValue<TFieldValues, TFieldName>>;
};

export type ControllerRenderProps<
  TFieldValues extends FieldValues = FieldValues,
  TName extends FieldPath<TFieldValues> = FieldPath<TFieldValues>,
> = {
  onChange: (...event: any[]) => void;
  onBlur: () => void;
  value: UnpackNestedValue<FieldPathValue<TFieldValues, TName>>;
  name: TName;
  ref: RefCallBack;
};

export type UseControllerProps<
  TFieldValues extends FieldValues = FieldValues,

Review comment (suggestion):
I didn't pay close enough attention here, @kotarella1110 this was completely changed after the merge.

Code after:
import * as React from 'react';

import { FieldErrors } from '..';

import { RegisterOptions } from './validator';
import {
  Control,
  FieldError,
  FieldPathValue,
  FieldPathWithValue,
  FieldValues,
  IsAny,
  NestedValue,
  Primitive,
  RefCallBack,
  UnpackNestedValue,
  UseFormStateReturn,
} from './';

type ControllerFieldError<
  TFieldValues extends FieldValues = FieldValues,
  TResult = any,
  TName extends FieldPathWithValue<TFieldValues, TResult> = FieldPathWithValue<
    TFieldValues,
    TResult
  >,
  FieldValuesAtPath = IsAny<TResult> extends true
    ? FieldPathValue<TFieldValues, TName>
    : TResult,
> = FieldValuesAtPath extends NestedValue | Primitive
  ? FieldError
  : FieldErrors<FieldValuesAtPath>;

export type ControllerFieldState<
  TFieldValues extends FieldValues = FieldValues,
  TResult = any,
  TName extends FieldPathWithValue<TFieldValues, TResult> = FieldPathWithValue<
    TFieldValues,
    TResult
  >,
> = {
  invalid: boolean;
  isTouched: boolean;
  isDirty: boolean;
  error?: ControllerFieldError<TFieldValues, TResult, TName>;
Repository: react-hook-form/react-hook-form
File: src/__tests__/controller.test.tsx (TypeScript)

Code before:
            render={({ field, fieldState }) => (
              <>
                <input {...field} />
                <p>{fieldState.error?.message}</p>
              </>
            )}
            control={control}
            name="deepNested.test"
          />
          <Controller
            render={({ field, fieldState }) => (
              <>
                <input {...field} />
                <p>{fieldState.error?.[0]?.message}</p>
              </>
            )}
            control={control}
            name="todos"
          />
        </form>
      );
    }

    render(<App />);

    expect(screen.getAllByRole('textbox'));
  });
});


Review comment (suggestion):
should this checking for 3 textboxes?  (just wondering if this might catch a textbox from somewhere else.)

The code itself looks good, just checking to see if we can strengthen this new test.

Code after:
            render={({ field, fieldState }) => (
              <>
                <input {...field} />
                <p>{fieldState.error?.message}</p>
              </>
            )}
            control={control}
            name="deepNested.test"
          />
          <Controller
            render={({ field, fieldState }) => (
              <>
                <input {...field} />
                <p>{fieldState.error?.[0]?.message}</p>
              </>
            )}
            control={control}
            name="todos"
          />
        </form>
      );
    }

    render(<App />);

    expect(screen.getAllByRole('textbox').length).toEqual(3);
  });
});
Repository: react-hook-form/react-hook-form
File: src/types/utils.ts (TypeScript)

Code before:
import { FieldValues } from './fields';
import { NestedValue } from './form';

export type Primitive =
  | null
  | undefined
  | string
  | number
  | boolean
  | symbol
  | bigint;

export type EmptyObject = { [K in string | number]: never };

export type NonUndefined<T> = T extends undefined ? never : T;

export type LiteralUnion<T extends U, U extends Primitive> =
  | T
  | (U & { _?: never });

type DeepPartialImpl<T> = T extends NestedValue
  ? T
  : T extends ReadonlyArray<any> | Record<any, unknown>
  ? DeepPartial<T>
  : T;

export type DeepPartial<T> = {
  [K in keyof T]?: DeepPartialImpl<T[K]>;
};

export type IsAny<T> = boolean extends (T extends never ? true : false)
  ? true
  : false;

export type DeepMapImpl<T, TValue> = IsAny<T> extends true
  ? any
  : T extends NestedValue
  ? TValue
  : T extends ReadonlyArray<any> | Record<any, unknown>
  ? DeepMap<T, TValue>
  : TValue;

export type DeepMap<T, TValue> = {
  [K in keyof T]: DeepMapImpl<NonUndefined<T[K]>, TValue>;
};


Review comment (suggestion):
this is not a concern but just a curiosity (I need to spend some time wrapping my head around all the latest typings) - is there a reason that NestedValue is in the form.ts file?

Seems like all the low level ones would fit in utils.ts (again no need to change, but just a generic comment)

Code after:
import { FieldValues } from './fields';
import { NestedValue } from './form';

export type Primitive =
  | null
  | undefined
  | string
  | number
  | boolean
  | symbol
  | bigint;

export type EmptyObject = { [K in string | number]: never };

export type NonUndefined<T> = T extends undefined ? never : T;

export type LiteralUnion<T extends U, U extends Primitive> =
  | T
  | (U & { _?: never });

export type DeepPartialImpl<T> = T extends NestedValue
  ? T
  : T extends ReadonlyArray<any> | Record<any, unknown>
  ? DeepPartial<T>
  : T;

export type DeepPartial<T> = {
  [K in keyof T]?: DeepPartialImpl<T[K]>;
};

export type IsAny<T> = boolean extends (T extends never ? true : false)
  ? true
  : false;

export type DeepMapImpl<T, TValue> = IsAny<T> extends true
  ? any
  : T extends NestedValue
  ? TValue
  : T extends ReadonlyArray<any> | Record<any, unknown>
  ? DeepMap<T, TValue>
  : TValue;

export type DeepMap<T, TValue> = {
  [K in keyof T]: DeepMapImpl<NonUndefined<T[K]>, TValue>;
};
Repository: react-hook-form/react-hook-form
File: src/utils/deepEqual.ts (TypeScript)

Code before:
import isObject from '../utils/isObject';

import isDateObject from './isDateObject';
import isPrimitive from './isPrimitive';

export default function deepEqual(object1: any, object2: any) {
  if (isPrimitive(object1) || isPrimitive(object2)) {
    return object1 === object2;
  }

  if (isDateObject(object1) && isDateObject(object2)) {
    return object1.getTime() === object2.getTime();
  }

  const keys1 = Object.keys(object1);
  const keys2 = Object.keys(object2);

  if (keys1.length !== keys2.length) {
    return false;
  }

  for (const key of keys1) {
    const val1 = object1[key];

    if (!keys2.includes(key)) {
      return false;
    }

    if (key !== 'ref') {
      const val2 = object2[key];

      if (
        (isDateObject(val1) && isDateObject(val2)) ||
        (isObject(val1) && isObject(val2)) ||
        (Array.isArray(val1) && Array.isArray(val2))
          ? !deepEqual(val1, val2)
          : val1 !== val2

Review comment (suggestion):
something bothers me about this - how does anything that's non null get past this code?  I get that we're avoiding the new Number equality case, but this is going to convert any set of nested objects by string?

I know you want to avoid other dependencies, but have you looked at something like https://www.npmjs.com/package/fast-deep-equal to do this.


Code after:
import isObject from '../utils/isObject';

import isDateObject from './isDateObject';
import isPrimitive from './isPrimitive';

export default function deepEqual(object1: any, object2: any) {
  if (isPrimitive(object1) || isPrimitive(object2)) {
    return object1 === object2;
  }

  if (!isNaN(object1) || !isNaN(object2)) {
    return object1.toString() === object2.toString();
  }

  if (isDateObject(object1) && isDateObject(object2)) {
    return object1.getTime() === object2.getTime();
  }

  const keys1 = Object.keys(object1);
  const keys2 = Object.keys(object2);

  if (keys1.length !== keys2.length) {
    return false;
  }

  for (const key of keys1) {
    const val1 = object1[key];

    if (!keys2.includes(key)) {
      return false;
    }

    if (key !== 'ref') {
      const val2 = object2[key];

      if (
        (isDateObject(val1) && isDateObject(val2)) ||
Repository: react-hook-form/react-hook-form
File: src/logic/createFormControl.ts (TypeScript)

Code before:
    } else {
      field && !field._f && !isNullOrUndefined(cloneValue)
        ? setValues(name, cloneValue, options)
        : setFieldValue(name, cloneValue, options);
    }

    isWatched(name, _names) && _subjects.state.next({});
    _subjects.watch.next({
      name,
    });
  };

  const onChange: ChangeHandler = async (event) => {
    const target = event.target;
    let name = target.name;
    const field: Field = get(_fields, name);

    if (field) {
      let error;
      let isValid;
      const fieldValue = target.type
        ? getFieldValue(field._f)
        : getEventValue(event);
      const isBlurEvent = [EVENTS.BLUR, EVENTS.FOCUS_OUT].some(
        (eventType) => event.type === eventType,
      );
      const shouldSkipValidation =
        (!hasValidation(field._f) &&
          !_options.resolver &&
          !get(_formState.errors, name) &&
          !field._f.deps) ||
        skipValidation(
          isBlurEvent,
          get(_formState.touchedFields, name),
          _formState.isSubmitted,
          validationModeAfterSubmit,
          validationModeBeforeSubmit,
        );
      const watched = isWatched(name, _names, isBlurEvent);

      set(_formValues, name, fieldValue);

      if (isBlurEvent) {
        field._f.onBlur && field._f.onBlur(event);
      } else if (field._f.onChange) {
        field._f.onChange(event);
      }

      const fieldState = updateTouchAndDirty(
        name,
        fieldValue,

Review comment (suggestion):
```suggestion
      const isBlurEvent = [EVENTS.BLUR, EVENTS.FOCUS_OUT].includes(event.type);
```

Save some bytes :) 

Code after:
    } else {
      field && !field._f && !isNullOrUndefined(cloneValue)
        ? setValues(name, cloneValue, options)
        : setFieldValue(name, cloneValue, options);
    }

    isWatched(name, _names) && _subjects.state.next({});
    _subjects.watch.next({
      name,
    });
  };

  const onChange: ChangeHandler = async (event) => {
    const target = event.target;
    let name = target.name;
    const field: Field = get(_fields, name);

    if (field) {
      let error;
      let isValid;
      const fieldValue = target.type
        ? getFieldValue(field._f)
        : getEventValue(event);
      const isBlurEvent =
        event.type === EVENTS.BLUR || event.type === EVENTS.FOCUS_OUT;
      const shouldSkipValidation =
        (!hasValidation(field._f) &&
          !_options.resolver &&
          !get(_formState.errors, name) &&
          !field._f.deps) ||
        skipValidation(
          isBlurEvent,
          get(_formState.touchedFields, name),
          _formState.isSubmitted,
          validationModeAfterSubmit,
          validationModeBeforeSubmit,
        );
      const watched = isWatched(name, _names, isBlurEvent);

      set(_formValues, name, fieldValue);

      if (isBlurEvent) {
        field._f.onBlur && field._f.onBlur(event);
      } else if (field._f.onChange) {
        field._f.onChange(event);
      }

      const fieldState = updateTouchAndDirty(
        name,
        fieldValue,
        isBlurEvent,
Repository: react-hook-form/react-hook-form
File: src/logic/createFormControl.ts (TypeScript)

Code before:
        });
      }
    } else {
      field && !field._f && !isNullOrUndefined(cloneValue)
        ? setValues(name, cloneValue, options)
        : setFieldValue(name, cloneValue, options);
    }

    isWatched(name, _names) && _subjects.state.next({});
    _subjects.watch.next({
      name,
    });
  };

  const onChange: ChangeHandler = async (event) => {
    const target = event.target;
    let name = target.name;
    const field: Field = get(_fields, name);

    if (field) {
      let error;
      let isValid;
      const fieldValue = target.type
        ? getFieldValue(field._f)
        : getEventValue(event);
      const isBlurEvent = [EVENTS.BLUR, EVENTS.FOCUS_OUT].includes(event.type);
      const shouldSkipValidation =
        (!hasValidation(field._f) &&
          !_options.resolver &&
          !get(_formState.errors, name) &&
          !field._f.deps) ||
        skipValidation(
          isBlurEvent,
          get(_formState.touchedFields, name),
          _formState.isSubmitted,
          validationModeAfterSubmit,
          validationModeBeforeSubmit,
        );
      const watched = isWatched(name, _names, isBlurEvent);

      set(_formValues, name, fieldValue);

      if (isBlurEvent) {
        field._f.onBlur && field._f.onBlur(event);
      } else if (field._f.onChange) {
        field._f.onChange(event);
      }

      const fieldState = updateTouchAndDirty(
        name,
        fieldValue,

Review comment (suggestion):
```suggestion
      const isBlurEvent = event.type === EVENTS.BLUR || event.type === EVENTS.FOCUS_OUT;
```

Code after:
        });
      }
    } else {
      field && !field._f && !isNullOrUndefined(cloneValue)
        ? setValues(name, cloneValue, options)
        : setFieldValue(name, cloneValue, options);
    }

    isWatched(name, _names) && _subjects.state.next({});
    _subjects.watch.next({
      name,
    });
  };

  const onChange: ChangeHandler = async (event) => {
    const target = event.target;
    let name = target.name;
    const field: Field = get(_fields, name);

    if (field) {
      let error;
      let isValid;
      const fieldValue = target.type
        ? getFieldValue(field._f)
        : getEventValue(event);
      const isBlurEvent =
        event.type === EVENTS.BLUR || event.type === EVENTS.FOCUS_OUT;
      const shouldSkipValidation =
        (!hasValidation(field._f) &&
          !_options.resolver &&
          !get(_formState.errors, name) &&
          !field._f.deps) ||
        skipValidation(
          isBlurEvent,
          get(_formState.touchedFields, name),
          _formState.isSubmitted,
          validationModeAfterSubmit,
          validationModeBeforeSubmit,
        );
      const watched = isWatched(name, _names, isBlurEvent);

      set(_formValues, name, fieldValue);

      if (isBlurEvent) {
        field._f.onBlur && field._f.onBlur(event);
      } else if (field._f.onChange) {
        field._f.onChange(event);
      }

      const fieldState = updateTouchAndDirty(
        name,