Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
2 tasks done
jlarmstrongiv opened this issue Mar 15, 2024 · 9 comments
Closed
2 tasks done

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

jlarmstrongiv opened this issue Mar 15, 2024 · 9 comments

Comments

@jlarmstrongiv
Copy link

jlarmstrongiv commented Mar 15, 2024

Is there an existing issue for this?

  • I have searched the existing issues and my issue is unique
  • 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

@ljharb
Copy link
Member

ljharb commented Mar 15, 2024

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
Copy link
Member

ljharb commented Mar 15, 2024

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

@jlarmstrongiv
Copy link
Author

jlarmstrongiv commented Mar 16, 2024

@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

@ljharb
Copy link
Member

ljharb commented Mar 16, 2024

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
Copy link
Contributor

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
Copy link
Member

ljharb commented Mar 16, 2024

@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
Copy link
Member

ljharb commented Jun 1, 2024

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 ljharb closed this as completed Jun 1, 2024
@jlarmstrongiv
Copy link
Author

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

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

@ljharb
Copy link
Member

ljharb commented Jun 6, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants