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]: react/prefer-read-only-props for Typescript #3535

Closed
2 tasks done
thany opened this issue Feb 22, 2023 · 3 comments · Fixed by #3593
Closed
2 tasks done

[Bug]: react/prefer-read-only-props for Typescript #3535

thany opened this issue Feb 22, 2023 · 3 comments · Fixed by #3593

Comments

@thany
Copy link

thany commented Feb 22, 2023

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

When I add react/prefer-read-only-props it doesn't detect read/write props defined using Typescript. The documentation suggests only towards the detection of types defined in Flow, so it makes sense (to me) that this rule can perform the same detection for Typescript as well.

Example of incorrect code:

interface ImageProps {
  src: string;
  alt?: string;
  width?: number;
  height?: number;
}

const Image: React.FC<ImageProps> = ({ src, alt, width, height }) => (
  <img src={src} alt={alt ?? ''} width={width} height={height} />
);

Examples of correct code:

interface ImageProps {
  readonly src: string;
  readonly alt?: string;
  readonly width?: number;
  readonly height?: number;
}

const Image: React.FC<ImageProps> = ({ src, alt, width, height }) => (
  <img src={src} alt={alt ?? ''} width={width} height={height} />
);
type ImageProps = ReadOnly<{
  src: string;
  alt?: string;
  width?: number;
  height?: number;
}>;

const Image: React.FC<ImageProps> = ({ src, alt, width, height }) => (
  <img src={src} alt={alt ?? ''} width={width} height={height} />
);
interface ImageProps {
  src: string;
  alt?: string;
  width?: number;
  height?: number;
}

const Image: React.FC<ReadOnly<ImageProps>> = ({ src, alt, width, height }) => (
  <img src={src} alt={alt ?? ''} width={width} height={height} />
);

I'm not sure what to do about advanced usage, so when pulling extra interfaces from other files, and unioning them into the ImageProps interface.

Expected Behavior

This rule should not only check Flow, but also Typescript.

Perhaps also a good idea to enforce how readonlyness is achieved (readonly keyword, ReadOnly<T> type, or React.FC<ReadOnly<T>> component, or something else?)

eslint-plugin-react version

7.32.2

eslint version

8.34.0

node version

18.13.0

@thany thany added the bug label Feb 22, 2023
@ljharb
Copy link
Member

ljharb commented Feb 23, 2023

This would be a rule that's only useful for those using TS or Flow - typically we've avoided rules that aren't useful to normal JS users.

@thany
Copy link
Author

thany commented Feb 23, 2023

Well, since the rule already exists, I thought we might as well make it work across both popular typed languages.

@ljharb
Copy link
Member

ljharb commented Feb 23, 2023

oh lol, fair enough. then yes, we should add/fix that support.

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

Successfully merging a pull request may close this issue.

2 participants