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

Code before:
              />
            ))}
          </div>
        );
      };

      const Component = () => {
        const { register, control } = useForm({
          defaultValues: {
            nest: {
              test: [
                { value: '1', nestedArray: [{ value: '2' }] },
                { value: '3', nestedArray: [{ value: '4' }] },
              ],
            },
          },
        });
        const { fields, remove, append } = useFieldArray({
          name: 'nest.test',
          control,
        });

        return (
          <div>
            {fields.map((item, i) => (
              <div key={item.value}>
                <input
                  name={`nest.test[${i}].value`}
                  ref={register()}
                  defaultValue={item.value}
                  placeholder="type"
                />

                <ChildComponent control={control} index={i} />

                <button
                  type={'button'}
                  onClick={() => remove(i)}
                  data-testid={item.value}
                >
                  remove
                </button>
              </div>
            ))}

            <button type={'button'} onClick={() => append({ value: 'test' })}>
              append
            </button>
          </div>
        );
      };

Review comment (suggestion):
```suggestion
              <div key={item.id}>
```

Code after:
              />
            ))}
          </div>
        );
      };

      const Component = () => {
        const { register, control } = useForm({
          defaultValues: {
            nest: {
              test: [
                { value: '1', nestedArray: [{ value: '2' }] },
                { value: '3', nestedArray: [{ value: '4' }] },
              ],
            },
          },
        });
        const { fields, remove, append } = useFieldArray({
          name: 'nest.test',
          control,
        });

        return (
          <div>
            {fields.map((item, i) => (
              <div key={item.id}>
                <input
                  name={`nest.test[${i}].value`}
                  ref={register()}
                  defaultValue={item.value}
                  placeholder="type"
                />

                <ChildComponent control={control} index={i} />

                <button
                  type={'button'}
                  onClick={() => remove(i)}
                  data-testid={item.value}
                >
                  remove
                </button>
              </div>
            ))}

            <button type={'button'} onClick={() => append({ value: 'test' })}>
              append
            </button>
          </div>
        );
      };
Repository: react-hook-form/react-hook-form
File: src/useFieldArray.test.tsx (TypeScript)

Code before:
                />

                <ChildComponent control={control} index={i} />

                <button
                  type={'button'}
                  onClick={() => remove(i)}
                  data-testid={item.value}
                >
                  remove
                </button>
              </div>
            ))}

            <button type={'button'} onClick={() => append({ value: 'test' })}>
              append
            </button>
          </div>
        );
      };

      render(<Component />);

      await actComponent(async () => {
        fireEvent.click(screen.getByTestId('1'));
      });

      expect(screen.getAllByPlaceholderText('type').length).toBe(2);

      await actComponent(async () => {
        fireEvent.click(screen.getByRole('button', { name: /append/i }));
      });

      expect(screen.getAllByPlaceholderText('type').length).toBe(3);
    });
  });
});


Review comment (suggestion):
```suggestion
      fireEvent.click(screen.getByTestId('1'));
```

I think this action is not async because this action trigger async method.(we should wrap `actComponent` with async if `mode` is `onChange`)

Code after:
                />

                <ChildComponent control={control} index={i} />

                <button
                  type={'button'}
                  onClick={() => remove(i)}
                  data-testid={item.value}
                >
                  remove
                </button>
              </div>
            ))}

            <button type={'button'} onClick={() => append({ value: 'test' })}>
              append
            </button>
          </div>
        );
      };

      render(<Component />);

      fireEvent.click(screen.getByTestId('1'));

      expect(screen.getAllByPlaceholderText('type')).toHaveLength(2);

      fireEvent.click(screen.getByRole('button', { name: /append/i }));

      expect(screen.getAllByPlaceholderText('type')).toHaveLength(3);
    });
  });
});
Repository: react-hook-form/react-hook-form
File: src/useFieldArray.test.tsx (TypeScript)

Code before:
                  onClick={() => remove(i)}
                  data-testid={item.value}
                >
                  remove
                </button>
              </div>
            ))}

            <button type={'button'} onClick={() => append({ value: 'test' })}>
              append
            </button>
          </div>
        );
      };

      render(<Component />);

      await actComponent(async () => {
        fireEvent.click(screen.getByTestId('1'));
      });

      expect(screen.getAllByPlaceholderText('type').length).toBe(2);

      await actComponent(async () => {
        fireEvent.click(screen.getByRole('button', { name: /append/i }));
      });

      expect(screen.getAllByPlaceholderText('type').length).toBe(3);
    });
  });
});


Review comment (suggestion):
```suggestion
      fireEvent.click(screen.getByRole('button', { name: /append/i }));
```

Code after:
                  onClick={() => remove(i)}
                  data-testid={item.value}
                >
                  remove
                </button>
              </div>
            ))}

            <button type={'button'} onClick={() => append({ value: 'test' })}>
              append
            </button>
          </div>
        );
      };

      render(<Component />);

      fireEvent.click(screen.getByTestId('1'));

      expect(screen.getAllByPlaceholderText('type')).toHaveLength(2);

      fireEvent.click(screen.getByRole('button', { name: /append/i }));

      expect(screen.getAllByPlaceholderText('type')).toHaveLength(3);
    });
  });
});
Repository: react-hook-form/react-hook-form
File: src/useFieldArray.test.tsx (TypeScript)

Code before:
                <ChildComponent control={control} index={i} />

                <button
                  type={'button'}
                  onClick={() => remove(i)}
                  data-testid={item.value}
                >
                  remove
                </button>
              </div>
            ))}

            <button type={'button'} onClick={() => append({ value: 'test' })}>
              append
            </button>
          </div>
        );
      };

      render(<Component />);

      await actComponent(async () => {
        fireEvent.click(screen.getByTestId('1'));
      });

      expect(screen.getAllByPlaceholderText('type').length).toBe(2);

      await actComponent(async () => {
        fireEvent.click(screen.getByRole('button', { name: /append/i }));
      });

      expect(screen.getAllByPlaceholderText('type').length).toBe(3);
    });
  });
});


Review comment (suggestion):
I think we should use `screen.getAllByRole('textbox')` because this test is using all `input` element. Specifying `'type'` to `placeholder` might make misunderstand this test use only specific element.

Code after:
                <ChildComponent control={control} index={i} />

                <button
                  type={'button'}
                  onClick={() => remove(i)}
                  data-testid={item.value}
                >
                  remove
                </button>
              </div>
            ))}

            <button type={'button'} onClick={() => append({ value: 'test' })}>
              append
            </button>
          </div>
        );
      };

      render(<Component />);

      fireEvent.click(screen.getByTestId('1'));

      expect(screen.getAllByPlaceholderText('type')).toHaveLength(2);

      fireEvent.click(screen.getByRole('button', { name: /append/i }));

      expect(screen.getAllByPlaceholderText('type')).toHaveLength(3);
    });
  });
});
Repository: react-hook-form/react-hook-form
File: src/useFieldArray.test.tsx (TypeScript)

Code before:
                <ChildComponent control={control} index={i} />

                <button
                  type={'button'}
                  onClick={() => remove(i)}
                  data-testid={item.value}
                >
                  remove
                </button>
              </div>
            ))}

            <button type={'button'} onClick={() => append({ value: 'test' })}>
              append
            </button>
          </div>
        );
      };

      render(<Component />);

      await actComponent(async () => {
        fireEvent.click(screen.getByTestId('1'));
      });

      expect(screen.getAllByPlaceholderText('type').length).toBe(2);

      await actComponent(async () => {
        fireEvent.click(screen.getByRole('button', { name: /append/i }));
      });

      expect(screen.getAllByPlaceholderText('type').length).toBe(3);
    });
  });
});


Review comment (suggestion):
```suggestion
      expect(screen.getAllByPlaceholderText('type')).toHaveLength(2);
```

Code after:
                <ChildComponent control={control} index={i} />

                <button
                  type={'button'}
                  onClick={() => remove(i)}
                  data-testid={item.value}
                >
                  remove
                </button>
              </div>
            ))}

            <button type={'button'} onClick={() => append({ value: 'test' })}>
              append
            </button>
          </div>
        );
      };

      render(<Component />);

      fireEvent.click(screen.getByTestId('1'));

      expect(screen.getAllByPlaceholderText('type')).toHaveLength(2);

      fireEvent.click(screen.getByRole('button', { name: /append/i }));

      expect(screen.getAllByPlaceholderText('type')).toHaveLength(3);
    });
  });
});
Repository: react-hook-form/react-hook-form
File: src/useFieldArray.test.tsx (TypeScript)

Code before:
                >
                  remove
                </button>
              </div>
            ))}

            <button type={'button'} onClick={() => append({ value: 'test' })}>
              append
            </button>
          </div>
        );
      };

      render(<Component />);

      await actComponent(async () => {
        fireEvent.click(screen.getByTestId('1'));
      });

      expect(screen.getAllByPlaceholderText('type').length).toBe(2);

      await actComponent(async () => {
        fireEvent.click(screen.getByRole('button', { name: /append/i }));
      });

      expect(screen.getAllByPlaceholderText('type').length).toBe(3);
    });
  });
});


Review comment (suggestion):
```suggestion
      expect(screen.getAllByPlaceholderText('type')).toHaveLength(3);
```

Code after:
                >
                  remove
                </button>
              </div>
            ))}

            <button type={'button'} onClick={() => append({ value: 'test' })}>
              append
            </button>
          </div>
        );
      };

      render(<Component />);

      fireEvent.click(screen.getByTestId('1'));

      expect(screen.getAllByPlaceholderText('type')).toHaveLength(2);

      fireEvent.click(screen.getByRole('button', { name: /append/i }));

      expect(screen.getAllByPlaceholderText('type')).toHaveLength(3);
    });
  });
});
Repository: react-hook-form/react-hook-form
File: src/useFieldArray.ts (TypeScript)

Code before:
      name,
      [],
    ),
  ];
  const memoizedDefaultValues = React.useRef<Partial<TFieldArrayValues>[]>(
    getDefaultValues(),
  );
  const [fields, setField] = React.useState<
    Partial<ArrayField<TFieldArrayValues, TKeyName>>[]
  >(mapIds(memoizedDefaultValues.current, keyName));
  const allFields = React.useRef<
    Partial<ArrayField<TFieldArrayValues, TKeyName>>[]
  >(fields);

  const getCurrentFieldsValues = () =>
    get(getValues() || {}, name, allFields.current).map(
      (item: Partial<TFieldArrayValues>, index: number) => ({
        ...allFields.current[index],
        ...item,
      }),
    );

  allFields.current = fields;

  if (process.env.NODE_ENV !== 'production') {
    if (!control) {
      console.warn('📋 useFieldArray is missing `control` prop.');
    }

    if (!name) {
      console.warn('📋 useFieldArray is missing `name` attribute.');
    }
  }

  const appendValueWithKey = (values: Partial<TFieldArrayValues>[]) =>
    values.map((value: Partial<TFieldArrayValues>) => appendId(value, keyName));

  const setFieldAndValidState = (
    fieldsValues: Partial<ArrayField<TFieldArrayValues, TKeyName>>[],
  ) => {
    setField(fieldsValues);

    if (readFormStateRef.current.isValid && validateSchemaIsValid) {
      validateSchemaIsValid({
        [name]: fieldsValues,
      });
    }
  };

  const shouldRenderFieldArray = (shouldRender?: boolean) => {
    if (

Review comment (suggestion):
```suggestion
    if (!control && methods.control) {
```

Code after:
      name,
      [],
    ),
  ];
  const memoizedDefaultValues = React.useRef<Partial<TFieldArrayValues>[]>(
    getDefaultValues(),
  );
  const [fields, setField] = React.useState<
    Partial<ArrayField<TFieldArrayValues, TKeyName>>[]
  >(mapIds(memoizedDefaultValues.current, keyName));
  const allFields = React.useRef<
    Partial<ArrayField<TFieldArrayValues, TKeyName>>[]
  >(fields);

  const getCurrentFieldsValues = () =>
    get(getValues() || {}, name, allFields.current).map(
      (item: Partial<TFieldArrayValues>, index: number) => ({
        ...allFields.current[index],
        ...item,
      }),
    );

  allFields.current = fields;

  if (process.env.NODE_ENV !== 'production') {
    if (!control && !methods.control) {
      console.warn('📋 useFieldArray is missing `control` prop.');
    }

    if (!name) {
      console.warn('📋 useFieldArray is missing `name` attribute.');
    }
  }

  const appendValueWithKey = (values: Partial<TFieldArrayValues>[]) =>
    values.map((value: Partial<TFieldArrayValues>) => appendId(value, keyName));

  const setFieldAndValidState = (
    fieldsValues: Partial<ArrayField<TFieldArrayValues, TKeyName>>[],
  ) => {
    setField(fieldsValues);

    if (readFormStateRef.current.isValid && validateSchemaIsValid) {
      validateSchemaIsValid({
        [name]: fieldsValues,
      });
    }
  };

  const shouldRenderFieldArray = (shouldRender?: boolean) => {
    if (
Repository: react-hook-form/react-hook-form
File: src/useForm.ts (TypeScript)

Code before:
                unset(errorsRef.current, name);
                validFieldsRef.current.add(name);
              }
            }
          }
        }

        if (
          isEmptyObject(fieldErrors) &&
          Object.keys(errorsRef.current).every((name) =>
            Object.keys(fieldsRef.current).includes(name),
          )
        ) {
          errorsRef.current = {};
          reRender();
          await callback(
            fieldValues as UnpackNestedValue<TSubmitFieldValues>,
            e,
          );
        } else {
          errorsRef.current = {
            ...errorsRef.current,
            ...fieldErrors,
          };
          if (onError) {
            await onError(fieldErrors);
          }
          if (shouldFocusError) {
            focusOnErrorField(fieldsRef.current, fieldErrors);
          }
        }
      } finally {
        isSubmittedRef.current = true;
        isSubmittingRef.current = false;
        submitCountRef.current = submitCountRef.current + 1;
        reRender();
      }
    },
    [shouldFocusError, isValidateAllFieldCriteria],
  );

  const resetRefs = ({
    errors,
    isDirty,
    isSubmitted,
    touched,
    isValid,
    submitCount,
    dirtyFields,
  }: OmitResetState) => {
    if (!errors) {

Review comment (suggestion):
I think you need to pass the second argument event here.

```
await onError(fieldErrors, e);
```

Code after:
                unset(errorsRef.current, name);
                validFieldsRef.current.add(name);
              }
            }
          }
        }

        if (
          isEmptyObject(fieldErrors) &&
          Object.keys(errorsRef.current).every((name) =>
            Object.keys(fieldsRef.current).includes(name),
          )
        ) {
          errorsRef.current = {};
          reRender();
          await onValid(
            fieldValues as UnpackNestedValue<TSubmitFieldValues>,
            e,
          );
        } else {
          errorsRef.current = {
            ...errorsRef.current,
            ...fieldErrors,
          };
          if (onInvalid) {
            await onInvalid(fieldErrors, e);
          }
          if (shouldFocusError) {
            focusOnErrorField(fieldsRef.current, fieldErrors);
          }
        }
      } finally {
        isSubmittedRef.current = true;
        isSubmittingRef.current = false;
        submitCountRef.current = submitCountRef.current + 1;
        reRender();
      }
    },
    [shouldFocusError, isValidateAllFieldCriteria],
  );

  const resetRefs = ({
    errors,
    isDirty,
    isSubmitted,
    touched,
    isValid,
    submitCount,
    dirtyFields,
  }: OmitResetState) => {
    if (!errors) {
Repository: react-hook-form/react-hook-form
File: rollup/createRollupConfig.js (JavaScript)

Code before:
      exports: 'named',
    },
    plugins: [
      external(),
      typescript({
        tsconfig: options.tsconfig,
        clean: true,
      }),
      resolve(),
      options.format === 'umd' &&
        commonjs({
          include: /\/node_modules\//,
        }),
      options.env !== undefined &&
        replace({
          'process.env.NODE_ENV': JSON.stringify(options.env),
        }),
      sourcemaps(),
      shouldMinify &&
        terser({
          output: { comments: false },
          compress: {
            drop_console: true,
          },
        }),
      ...(options.plugins || []),
    ],
  };
}


Review comment (suggestion):
It doesn't seem to be minify.
Must be after rollup-plugin-typescript2 in the plugin list.

<img width="1009" alt="スクリーンショット 2020-08-11 12 04 11" src="https://user-images.githubusercontent.com/12913947/89852880-d57f7c00-dbca-11ea-853c-bfe56b9bdcd9.png">


Code after:
      exports: 'named',
    },
    plugins: [
      external(),
      typescript({
        tsconfig: options.tsconfig,
        clean: true,
      }),
      resolve(),
      options.format === 'umd' &&
        commonjs({
          include: /\/node_modules\//,
        }),
      options.env !== undefined &&
        replace({
          'process.env.NODE_ENV': JSON.stringify(options.env),
        }),
      sourcemaps(),
      shouldMinify &&
        terser({
          output: { comments: false },
          compress: {
            drop_console: true,
          },
        }),
    ].filter(Boolean),
  };

  return callback ? callback(config) : config;
}
Repository: react-hook-form/react-hook-form
File: src/useForm.ts (TypeScript)

Code before:
              inputRef.closest('form')!.reset();
              break;
            } catch {}
          }
        }
      }
    }

    fieldsRef.current = {};

    defaultValuesRef.current = values || { ...defaultValuesRef.current };

    if (values) {
      renderWatchedInputs('');
    }

    unmountFieldsStateRef.current = shouldUnregister ? {} : values || {};

    Object.values(resetFieldArrayFunctionRef.current).forEach(
      (resetFieldArray) => isFunction(resetFieldArray) && resetFieldArray(),
    );

    resetRefs(omitResetState);
  };

  observerRef.current = observerRef.current
    ? observerRef.current
    : onDomRemove(fieldsRef, removeFieldEventListenerAndRef);

  React.useEffect(() => {
    isUnMount.current = false;

    return () => {
      isUnMount.current = true;

      if (observerRef.current) {
        observerRef.current.disconnect();
      }

      if (process.env.NODE_ENV !== 'production') {
        return;
      }

      fieldsRef.current &&
        Object.values(fieldsRef.current).forEach((field) =>
          removeFieldEventListenerAndRef(field, true),
        );
    };
  }, [removeFieldEventListenerAndRef]);

  if (!resolver && readFormStateRef.current.isValid) {

Review comment (suggestion):
nb: maybe `useMemo` is more suitable option:

```js
const observer = React.useMemo(
  () => onDomRemove(fieldsRef, removeFieldEventListenerAndRef),
  [fieldsRef, removeFieldEventListenerAndRef]
);
```

Code after:

    defaultValuesRef.current = values || { ...defaultValuesRef.current };

    if (values) {
      renderWatchedInputs('');
    }

    unmountFieldsStateRef.current = shouldUnregister ? {} : values || {};

    Object.values(resetFieldArrayFunctionRef.current).forEach(
      (resetFieldArray) => isFunction(resetFieldArray) && resetFieldArray(),
    );

    resetRefs(omitResetState);
  };

  observerRef.current =
    observerRef.current || !isWeb
      ? observerRef.current
      : onDomRemove(fieldsRef, removeFieldEventListenerAndRef);

  React.useEffect(() => {
    isUnMount.current = false;

    return () => {
      isUnMount.current = true;

      if (observerRef.current) {
        observerRef.current.disconnect();
      }

      if (process.env.NODE_ENV !== 'production') {
        return;
      }

      fieldsRef.current &&
        Object.values(fieldsRef.current).forEach((field) =>
          removeFieldEventListenerAndRef(field, true),
        );
    };
  }, [removeFieldEventListenerAndRef]);

  if (!resolver && readFormStateRef.current.isValid) {
    formState.isValid =
      deepEqual(validFieldsRef.current, fieldsWithValidationRef.current) &&
      isEmptyObject(formState.errors);
  }

  const commonProps = {
    trigger,
    setValue: React.useCallback(setValue, [setInternalValue, trigger]),