jsx-eslint / eslint-plugin-react

React-specific linting rules for ESLint
MIT License
8.97k stars 2.77k forks source link

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

Closed jlarmstrongiv closed 4 months ago

jlarmstrongiv commented 6 months ago

Is there an existing issue for this?

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

ljharb commented 6 months ago

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 commented 6 months ago

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

jlarmstrongiv commented 6 months ago

@ljharb I’m using @typescript-eslint/parser@7.2.0 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

ljharb commented 6 months ago

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.

developer-bandi commented 6 months ago

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.

ljharb commented 6 months ago

@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 commented 4 months ago

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.

jlarmstrongiv commented 3 months ago

@ljharb would you re-open this issue? I am still getting the error in eslint-plugin-react@7.34.2:

TypeError: Cannot read properties of undefined (reading 'properties')
ljharb commented 3 months ago

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