eslint-plugin-react icon indicating copy to clipboard operation
eslint-plugin-react copied to clipboard

[Bug]: `boolean-prop-naming` rule throws error

Open jlarmstrongiv opened this issue 11 months ago • 6 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues and my issue is unique
  • [X] My issue appears in the command-line and not only in the text editor

Description Overview

eslint-plugin-react breaks in a patch update.

I wasn’t able to narrow it down, but @developer-bandi and @ljharb made changes to the boolean-prop-naming in the current release. While the fixes are fantastic, it seems like something also broke in the process.

From https://kitchen-sink.trpc.io/react-hook-form

import { zodResolver } from "@hookform/resolvers/zod";
import { useId } from "react";
import type {
  FieldValues,
  SubmitHandler,
  UseFormProps,
  UseFormReturn,
} from "react-hook-form";
import { FormProvider, useForm, useFormContext } from "react-hook-form";
import type { z } from "zod";
import { errorMap } from "zod-validation-error";

// reference https://kitchen-sink.trpc.io/react-hook-form?file=feature%2Freact-hook-form%2FForm.tsx#content

export type UseZodForm<TInput extends FieldValues> = UseFormReturn<TInput> & {
  /**
   * A unique ID for this form.
   */
  id: string;
};
export function useZodForm<TSchema extends z.ZodType>(
  props: Omit<UseFormProps<TSchema["_input"]>, "resolver"> & {
    schema: TSchema;
  },
): UseZodForm<TSchema["_input"]> {
  const form = useForm<TSchema["_input"]>({
    mode: "onTouched",
    ...props,
    resolver: zodResolver(
      props.schema,
      {
        async: true,
        errorMap,
      },
      {
        // This makes it so we can use `.transform()`s on the schema without same transform getting applied again when it reaches the server
        raw: true,
      },
    ),
  }) as UseZodForm<TSchema["_input"]>;

  form.id = useId();

  return form;
}

export type AnyZodForm = UseZodForm<any>;

export function Form<TInput extends FieldValues>(
  props: Omit<React.ComponentProps<"form">, "id" | "onSubmit"> & {
    readonly form: UseZodForm<TInput>;
    readonly handleSubmit: SubmitHandler<TInput>;
  },
): JSX.Element {
  const { form, handleSubmit, ...passThrough }: typeof props = props;
  return (
    <FormProvider {...form}>
      <form
        {...passThrough}
        id={form.id}
        onSubmit={(event) => {
          form.handleSubmit(async (values) => {
            try {
              await handleSubmit(values);
            } catch (error) {
              form.setError("root.server", {
                message: (error as Error)?.message ?? "Unknown error",
                type: "server",
              });
            }
          })(event);
        }}
      />
    </FormProvider>
  );
}

export function SubmitButton(
  props: Omit<React.ComponentProps<"button">, "form" | "type"> & {
    /**
     * Optionally specify a form to submit instead of the closest form context.
     */
    readonly form?: AnyZodForm;
  },
): JSX.Element {
  const context = useFormContext();

  const form = props.form ?? context;
  if (!form) {
    throw new Error(
      "SubmitButton must be used within a Form or have a form prop",
    );
  }

  const { formState } = form;

  return (
    <button
      {...props}
      disabled={formState.isSubmitting}
      form={props.form?.id}
      type="submit"
    >
      {formState.isSubmitting ? "Loading" : props.children}
    </button>
  );
}

export function ResetButton(
  props: Omit<React.ComponentProps<"button">, "form" | "type"> & {
    /**
     * Optionally specify a form to submit instead of the closest form context.
     */
    readonly form?: AnyZodForm;
  },
): JSX.Element {
  const context = useFormContext();

  const form = props.form ?? context;
  if (!form) {
    throw new Error(
      "SubmitButton must be used within a Form or have a form prop",
    );
  }

  const { formState } = form;

  return (
    <button
      {...props}
      disabled={formState.isSubmitting}
      form={props.form?.id}
      type="reset"
    >
      {props.children}
    </button>
  );
}
Oops! Something went wrong! :(

ESLint: 8.57.0

TypeError: Cannot read properties of undefined (reading 'name')
Occurred while linting /home/runner/actions-runner/_work/project-name/project-name/packages/core/src/form.tsx:1
Rule: "react/boolean-prop-naming"
    at /home/runner/actions-runner/_work/project-name/project-name/node_modules/eslint-plugin-react/lib/rules/boolean-prop-naming.js:389:102
    at Array.flatMap (<anonymous>)
    at /home/runner/actions-runner/_work/project-name/project-name/node_modules/eslint-plugin-react/lib/rules/boolean-prop-naming.js:389:26
    at Array.forEach (<anonymous>)
    at Program:exit (/home/runner/actions-runner/_work/project-name/project-name/node_modules/eslint-plugin-react/lib/rules/boolean-prop-naming.js:377:35)
    at /home/runner/actions-runner/_work/project-name/project-name/node_modules/eslint-plugin-react/lib/util/Components.js:277:9
    at Array.forEach (<anonymous>)
    at mergedHandler (/home/runner/actions-runner/_work/project-name/project-name/node_modules/eslint-plugin-react/lib/util/Components.js:276:16)
    at ruleErrorHandler (/home/runner/actions-runner/_work/project-name/project-name/node_modules/eslint/lib/linter/linter.js:1076:28)
    at /home/runner/actions-runner/_work/project-name/project-name/node_modules/eslint/lib/linter/safe-emitter.js:45:58
Error: Process completed with exit code 2.

npx eslint . --ext .ts,.tsx,.js,.jsx --ignore-path=".gitignore"

Expected Behavior

eslint telling me a rule was broken, not crashing unexpectedly

eslint-plugin-react version

7.34.1

eslint version

8.57.0

node version

20.11.1

jlarmstrongiv avatar Mar 15 '24 23:03 jlarmstrongiv

Thanks for the report! unfortunately the tests pass for me. I see an easy fix but I can't land it without a failing test case :-/

ljharb avatar Mar 15 '24 23:03 ljharb

what version of the TS eslint parser do you have installed?

ljharb avatar Mar 15 '24 23:03 ljharb

@ljharb I’m using @typescript-eslint/[email protected] thanks for looking into it!

Not sure why it’s passing, but the only difference in my project is updating eslint-plugin-react, so it has to be one of the changes in 7.34.1

jlarmstrongiv avatar Mar 16 '24 01:03 jlarmstrongiv

oh it’s definitely a change in the react plugin :-) but we don’t even test on v6 of that parser, let alone v7 (which i didn’t know exists yet) which is why we didn’t catch it.

ljharb avatar Mar 16 '24 02:03 ljharb

There may be an impact due to the fact that typescript version 7 has not been tested, but it is assumed that the case below causes the above error.

type Props = {
  enabled: boolean
}
      
const Hello = (props: Props & {
  semi: boolean
}) => <div />;

So first, i modify the code so that the test case succeeds.

developer-bandi avatar Mar 16 '24 02:03 developer-bandi

@developer-bandi just make sure the test fails before the fix is applied :-)

we may need to add testing in v6 and v7 to reproduce it in CI.

ljharb avatar Mar 16 '24 03:03 ljharb

I'm going to close this, and assume it's fixed by #3718 and the commit that closed #3733.

After the next release, if it's still happening, I'll reopen.

ljharb avatar Jun 01 '24 01:06 ljharb

@ljharb would you re-open this issue? I am still getting the error in [email protected]:

TypeError: Cannot read properties of undefined (reading 'properties')

jlarmstrongiv avatar Jun 06 '24 18:06 jlarmstrongiv

@jlarmstrongiv that's because the fix isn't released yet. Issues on github are closed when fixes are landed, not when they're released.

ljharb avatar Jun 06 '24 18:06 ljharb