Skip to content

Commit

Permalink
[Fix] prefer-read-only-props: add TS support
Browse files Browse the repository at this point in the history
  • Loading branch information
HenryBrown0 authored and ljharb committed Jun 28, 2023
1 parent fa1c277 commit 9c5ac98
Show file tree
Hide file tree
Showing 4 changed files with 264 additions and 35 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,14 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
* [`no-unused-state`]: avoid crashing on a class field function with destructured state ([#3568][] @ljharb)
* [`no-unused-prop-types`]: allow using spread with object expression in jsx ([#3570][] @akulsr0)
* Revert "[`destructuring-assignment`]: Handle destructuring of useContext in SFC" ([#3583][] [#2797][] @102)
* [`prefer-read-only-props`]: add TS support ([#3593][] @HenryBrown0)

### Changed
* [Docs] [`jsx-newline`], [`no-unsafe`], [`static-property-placement`]: Fix code syntax highlighting ([#3563][] @nbsp1221)
* [readme] resore configuration URL ([#3582][] @gokaygurcan)
* [Docs] [`jsx-no-bind`]: reword performance rationale ([#3581][] @gpoole)

[#3593]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3593
[#3583]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3583
[#3582]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3582
[#3581]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3581
Expand Down
48 changes: 48 additions & 0 deletions docs/rules/prefer-read-only-props.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ Using Flow, one can define types for props. This rule enforces that prop types a

Examples of **incorrect** code for this rule:

In Flow:

```jsx
type Props = {
name: string,
Expand All @@ -29,8 +31,32 @@ const Hello = (props: {|name: string|}) => (
);
```

In TypeScript:

```tsx
type Props = {
name: string;
}
class Hello extends React.Component<Props> {
render () {
return <div>Hello {this.props.name}</div>;
}
}

interface Props {
name: string;
}
class Hello extends React.Component<Props> {
render () {
return <div>Hello {this.props.name}</div>;
}
}
```

Examples of **correct** code for this rule:

In Flow:

```jsx
type Props = {
+name: string,
Expand All @@ -49,3 +75,25 @@ const Hello = (props: {|+name: string|}) => (
<div>Hello {props.name}</div>
);
```

In TypeScript:

```tsx
type Props = {
readonly name: string;
}
class Hello extends React.Component<Props> {
render () {
return <div>Hello {this.props.name}</div>;
}
}

interface Props {
readonly name: string;
}
class Hello extends React.Component<Props> {
render () {
return <div>Hello {this.props.name}</div>;
}
}
```
95 changes: 62 additions & 33 deletions lib/rules/prefer-read-only-props.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ function isFlowPropertyType(node) {
return node.type === 'ObjectTypeProperty';
}

function isTypescriptPropertyType(node) {
return node.type === 'TSPropertySignature';
}

function isCovariant(node) {
return (node.variance && node.variance.kind === 'plus')
|| (
Expand All @@ -27,6 +31,14 @@ function isCovariant(node) {
);
}

function isReadonly(node) {
return (
node.typeAnnotation
&& node.typeAnnotation.parent
&& node.typeAnnotation.parent.readonly

Check warning on line 38 in lib/rules/prefer-read-only-props.js

View check run for this annotation

Codecov / codecov/patch

lib/rules/prefer-read-only-props.js#L35-L38

Added lines #L35 - L38 were not covered by tests
);
}

// ------------------------------------------------------------------------------
// Rule Definition
// ------------------------------------------------------------------------------
Expand All @@ -50,38 +62,55 @@ module.exports = {
schema: [],
},

create: Components.detect((context, components) => ({
'Program:exit'() {
flatMap(
values(components.list()),
(component) => component.declaredPropTypes || []
).forEach((declaredPropTypes) => {
Object.keys(declaredPropTypes).forEach((propName) => {
const prop = declaredPropTypes[propName];

if (!prop.node || !isFlowPropertyType(prop.node)) {
return;
}

if (!isCovariant(prop.node)) {
report(context, messages.readOnlyProp, 'readOnlyProp', {
node: prop.node,
data: {
name: propName,
},
fix: (fixer) => {
if (!prop.node.variance) {
// Insert covariance
return fixer.insertTextBefore(prop.node, '+');
}

// Replace contravariance with covariance
return fixer.replaceText(prop.node.variance, '+');
},
});
}
});
create: Components.detect((context, components) => {
function reportReadOnlyProp(prop, propName, fixer) {
report(context, messages.readOnlyProp, 'readOnlyProp', {
node: prop.node,
data: {
name: propName,
},
fix: fixer,
});
},
})),
}

return {
'Program:exit'() {
flatMap(
values(components.list()),
(component) => component.declaredPropTypes || []
).forEach((declaredPropTypes) => {
Object.keys(declaredPropTypes).forEach((propName) => {
const prop = declaredPropTypes[propName];
if (!prop.node) {
return;

Check warning on line 85 in lib/rules/prefer-read-only-props.js

View check run for this annotation

Codecov / codecov/patch

lib/rules/prefer-read-only-props.js#L85

Added line #L85 was not covered by tests
}

if (isFlowPropertyType(prop.node)) {
if (!isCovariant(prop.node)) {
reportReadOnlyProp(prop, propName, (fixer) => {
if (!prop.node.variance) {
// Insert covariance
return fixer.insertTextBefore(prop.node, '+');
}

// Replace contravariance with covariance
return fixer.replaceText(prop.node.variance, '+');
});
}

return;
}

if (isTypescriptPropertyType(prop.node)) {
if (!isReadonly(prop.node)) {
reportReadOnlyProp(prop, propName, (fixer) => (
fixer.insertTextBefore(prop.node, 'readonly ')

Check warning on line 107 in lib/rules/prefer-read-only-props.js

View check run for this annotation

Codecov / codecov/patch

lib/rules/prefer-read-only-props.js#L105-L107

Added lines #L105 - L107 were not covered by tests
));
}
}
});
});
},
};
}),
};
154 changes: 152 additions & 2 deletions tests/lib/rules/prefer-read-only-props.js
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ ruleTester.run('prefer-read-only-props', rule, {
import React from "react";
interface Props {
name: string;
readonly name: string;
}
const MyComponent: React.FC<Props> = ({ name }) => {
Expand All @@ -176,7 +176,62 @@ ruleTester.run('prefer-read-only-props', rule, {
export default MyComponent;
`,
features: ['ts'],
features: ['ts', 'no-babel-old'],
},
{
code: `
import React from "react";
type Props = {
readonly firstName: string;
readonly lastName: string;
}
const MyComponent: React.FC<Props> = ({ name }) => {
return <div>{name}</div>;
};
export default MyComponent;
`,
features: ['ts', 'no-babel-old'],
},
{
code: `
import React from "react";
type Props = {
readonly name: string;
}
const MyComponent: React.FC<Props> = ({ name }) => {
return <div>{name}</div>;
};
export default MyComponent;
`,
features: ['ts', 'no-babel-old'],
},
{
code: `
import React from "react";
type Props = {
readonly name: string[];
}
const MyComponent: React.FC<Props> = ({ name }) => {
return <div>{name}</div>;
};
export default MyComponent;
`,
features: ['ts', 'no-babel-old'],
},
{
code: `
import React from "react";
type Props = {
readonly person: {
name: string;
}
}
const MyComponent: React.FC<Props> = ({ name }) => {
return <div>{name}</div>;
};
export default MyComponent;
`,
features: ['ts', 'no-babel-old'],
},
]),

Expand Down Expand Up @@ -383,5 +438,100 @@ ruleTester.run('prefer-read-only-props', rule, {
},
],
},
{
code: `
type Props = {
name: string;
}
class Hello extends React.Component<Props> {
render () {
return <div>Hello {this.props.name}</div>;
}
}
`,
output: `
type Props = {
readonly name: string;
}
class Hello extends React.Component<Props> {
render () {
return <div>Hello {this.props.name}</div>;
}
}
`,
features: ['ts', 'no-babel-old'],
errors: [
{
messageId: 'readOnlyProp',
data: { name: 'name' },
},
],
},
{
code: `
interface Props {
name: string;
}
class Hello extends React.Component<Props> {
render () {
return <div>Hello {this.props.name}</div>;
}
}
`,
output: `
interface Props {
readonly name: string;
}
class Hello extends React.Component<Props> {
render () {
return <div>Hello {this.props.name}</div>;
}
}
`,
features: ['ts', 'no-babel-old'],
errors: [
{
messageId: 'readOnlyProp',
data: { name: 'name' },
},
],
},
{
code: `
type Props = {
readonly firstName: string;
lastName: string;
}
class Hello extends React.Component<Props> {
render () {
return <div>Hello {this.props.name}</div>;
}
}
`,
output: `
type Props = {
readonly firstName: string;
readonly lastName: string;
}
class Hello extends React.Component<Props> {
render () {
return <div>Hello {this.props.name}</div>;
}
}
`,
features: ['ts', 'no-babel-old'],
errors: [
{
messageId: 'readOnlyProp',
data: { name: 'lastName' },
},
],
},
]),
});

0 comments on commit 9c5ac98

Please sign in to comment.