Skip to content

Commit 9c5ac98

Browse files
HenryBrown0ljharb
HenryBrown0
authored and
ljharb
committed
[Fix] prefer-read-only-props: add TS support
1 parent fa1c277 commit 9c5ac98

4 files changed

Lines changed: 264 additions & 35 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,14 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
2121
* [`no-unused-state`]: avoid crashing on a class field function with destructured state ([#3568][] @ljharb)
2222
* [`no-unused-prop-types`]: allow using spread with object expression in jsx ([#3570][] @akulsr0)
2323
* Revert "[`destructuring-assignment`]: Handle destructuring of useContext in SFC" ([#3583][] [#2797][] @102)
24+
* [`prefer-read-only-props`]: add TS support ([#3593][] @HenryBrown0)
2425

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

31+
[#3593]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3593
3032
[#3583]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3583
3133
[#3582]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3582
3234
[#3581]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3581

‎docs/rules/prefer-read-only-props.md‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ Using Flow, one can define types for props. This rule enforces that prop types a
1010

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

13+
In Flow:
14+
1315
```jsx
1416
type Props = {
1517
name: string,
@@ -29,8 +31,32 @@ const Hello = (props: {|name: string|}) => (
2931
);
3032
```
3133

34+
In TypeScript:
35+
36+
```tsx
37+
type Props = {
38+
name: string;
39+
}
40+
class Hello extends React.Component<Props> {
41+
render () {
42+
return <div>Hello {this.props.name}</div>;
43+
}
44+
}
45+
46+
interface Props {
47+
name: string;
48+
}
49+
class Hello extends React.Component<Props> {
50+
render () {
51+
return <div>Hello {this.props.name}</div>;
52+
}
53+
}
54+
```
55+
3256
Examples of **correct** code for this rule:
3357

58+
In Flow:
59+
3460
```jsx
3561
type Props = {
3662
+name: string,
@@ -49,3 +75,25 @@ const Hello = (props: {|+name: string|}) => (
4975
<div>Hello {props.name}</div>
5076
);
5177
```
78+
79+
In TypeScript:
80+
81+
```tsx
82+
type Props = {
83+
readonly name: string;
84+
}
85+
class Hello extends React.Component<Props> {
86+
render () {
87+
return <div>Hello {this.props.name}</div>;
88+
}
89+
}
90+
91+
interface Props {
92+
readonly name: string;
93+
}
94+
class Hello extends React.Component<Props> {
95+
render () {
96+
return <div>Hello {this.props.name}</div>;
97+
}
98+
}
99+
```

‎lib/rules/prefer-read-only-props.js‎

Lines changed: 62 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@ function isFlowPropertyType(node) {
1616
return node.type === 'ObjectTypeProperty';
1717
}
1818

19+
function isTypescriptPropertyType(node) {
20+
return node.type === 'TSPropertySignature';
21+
}
22+
1923
function isCovariant(node) {
2024
return (node.variance && node.variance.kind === 'plus')
2125
|| (
@@ -27,6 +31,14 @@ function isCovariant(node) {
2731
);
2832
}
2933

34+
function isReadonly(node) {
35+
return (
36+
node.typeAnnotation
37+
&& node.typeAnnotation.parent
38+
&& node.typeAnnotation.parent.readonly
39+
);
40+
}
41+
3042
// ------------------------------------------------------------------------------
3143
// Rule Definition
3244
// ------------------------------------------------------------------------------
@@ -50,38 +62,55 @@ module.exports = {
5062
schema: [],
5163
},
5264

53-
create: Components.detect((context, components) => ({
54-
'Program:exit'() {
55-
flatMap(
56-
values(components.list()),
57-
(component) => component.declaredPropTypes || []
58-
).forEach((declaredPropTypes) => {
59-
Object.keys(declaredPropTypes).forEach((propName) => {
60-
const prop = declaredPropTypes[propName];
61-
62-
if (!prop.node || !isFlowPropertyType(prop.node)) {
63-
return;
64-
}
65-
66-
if (!isCovariant(prop.node)) {
67-
report(context, messages.readOnlyProp, 'readOnlyProp', {
68-
node: prop.node,
69-
data: {
70-
name: propName,
71-
},
72-
fix: (fixer) => {
73-
if (!prop.node.variance) {
74-
// Insert covariance
75-
return fixer.insertTextBefore(prop.node, '+');
76-
}
77-
78-
// Replace contravariance with covariance
79-
return fixer.replaceText(prop.node.variance, '+');
80-
},
81-
});
82-
}
83-
});
65+
create: Components.detect((context, components) => {
66+
function reportReadOnlyProp(prop, propName, fixer) {
67+
report(context, messages.readOnlyProp, 'readOnlyProp', {
68+
node: prop.node,
69+
data: {
70+
name: propName,
71+
},
72+
fix: fixer,
8473
});
85-
},
86-
})),
74+
}
75+
76+
return {
77+
'Program:exit'() {
78+
flatMap(
79+
values(components.list()),
80+
(component) => component.declaredPropTypes || []
81+
).forEach((declaredPropTypes) => {
82+
Object.keys(declaredPropTypes).forEach((propName) => {
83+
const prop = declaredPropTypes[propName];
84+
if (!prop.node) {
85+
return;
86+
}
87+
88+
if (isFlowPropertyType(prop.node)) {
89+
if (!isCovariant(prop.node)) {
90+
reportReadOnlyProp(prop, propName, (fixer) => {
91+
if (!prop.node.variance) {
92+
// Insert covariance
93+
return fixer.insertTextBefore(prop.node, '+');
94+
}
95+
96+
// Replace contravariance with covariance
97+
return fixer.replaceText(prop.node.variance, '+');
98+
});
99+
}
100+
101+
return;
102+
}
103+
104+
if (isTypescriptPropertyType(prop.node)) {
105+
if (!isReadonly(prop.node)) {
106+
reportReadOnlyProp(prop, propName, (fixer) => (
107+
fixer.insertTextBefore(prop.node, 'readonly ')
108+
));
109+
}
110+
}
111+
});
112+
});
113+
},
114+
};
115+
}),
87116
};

‎tests/lib/rules/prefer-read-only-props.js‎

Lines changed: 152 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,7 @@ ruleTester.run('prefer-read-only-props', rule, {
167167
import React from "react";
168168
169169
interface Props {
170-
name: string;
170+
readonly name: string;
171171
}
172172
173173
const MyComponent: React.FC<Props> = ({ name }) => {
@@ -176,7 +176,62 @@ ruleTester.run('prefer-read-only-props', rule, {
176176
177177
export default MyComponent;
178178
`,
179-
features: ['ts'],
179+
features: ['ts', 'no-babel-old'],
180+
},
181+
{
182+
code: `
183+
import React from "react";
184+
type Props = {
185+
readonly firstName: string;
186+
readonly lastName: string;
187+
}
188+
const MyComponent: React.FC<Props> = ({ name }) => {
189+
return <div>{name}</div>;
190+
};
191+
export default MyComponent;
192+
`,
193+
features: ['ts', 'no-babel-old'],
194+
},
195+
{
196+
code: `
197+
import React from "react";
198+
type Props = {
199+
readonly name: string;
200+
}
201+
const MyComponent: React.FC<Props> = ({ name }) => {
202+
return <div>{name}</div>;
203+
};
204+
export default MyComponent;
205+
`,
206+
features: ['ts', 'no-babel-old'],
207+
},
208+
{
209+
code: `
210+
import React from "react";
211+
type Props = {
212+
readonly name: string[];
213+
}
214+
const MyComponent: React.FC<Props> = ({ name }) => {
215+
return <div>{name}</div>;
216+
};
217+
export default MyComponent;
218+
`,
219+
features: ['ts', 'no-babel-old'],
220+
},
221+
{
222+
code: `
223+
import React from "react";
224+
type Props = {
225+
readonly person: {
226+
name: string;
227+
}
228+
}
229+
const MyComponent: React.FC<Props> = ({ name }) => {
230+
return <div>{name}</div>;
231+
};
232+
export default MyComponent;
233+
`,
234+
features: ['ts', 'no-babel-old'],
180235
},
181236
]),
182237

@@ -383,5 +438,100 @@ ruleTester.run('prefer-read-only-props', rule, {
383438
},
384439
],
385440
},
441+
{
442+
code: `
443+
type Props = {
444+
name: string;
445+
}
446+
447+
class Hello extends React.Component<Props> {
448+
render () {
449+
return <div>Hello {this.props.name}</div>;
450+
}
451+
}
452+
`,
453+
output: `
454+
type Props = {
455+
readonly name: string;
456+
}
457+
458+
class Hello extends React.Component<Props> {
459+
render () {
460+
return <div>Hello {this.props.name}</div>;
461+
}
462+
}
463+
`,
464+
features: ['ts', 'no-babel-old'],
465+
errors: [
466+
{
467+
messageId: 'readOnlyProp',
468+
data: { name: 'name' },
469+
},
470+
],
471+
},
472+
{
473+
code: `
474+
interface Props {
475+
name: string;
476+
}
477+
478+
class Hello extends React.Component<Props> {
479+
render () {
480+
return <div>Hello {this.props.name}</div>;
481+
}
482+
}
483+
`,
484+
output: `
485+
interface Props {
486+
readonly name: string;
487+
}
488+
489+
class Hello extends React.Component<Props> {
490+
render () {
491+
return <div>Hello {this.props.name}</div>;
492+
}
493+
}
494+
`,
495+
features: ['ts', 'no-babel-old'],
496+
errors: [
497+
{
498+
messageId: 'readOnlyProp',
499+
data: { name: 'name' },
500+
},
501+
],
502+
},
503+
{
504+
code: `
505+
type Props = {
506+
readonly firstName: string;
507+
lastName: string;
508+
}
509+
510+
class Hello extends React.Component<Props> {
511+
render () {
512+
return <div>Hello {this.props.name}</div>;
513+
}
514+
}
515+
`,
516+
output: `
517+
type Props = {
518+
readonly firstName: string;
519+
readonly lastName: string;
520+
}
521+
522+
class Hello extends React.Component<Props> {
523+
render () {
524+
return <div>Hello {this.props.name}</div>;
525+
}
526+
}
527+
`,
528+
features: ['ts', 'no-babel-old'],
529+
errors: [
530+
{
531+
messageId: 'readOnlyProp',
532+
data: { name: 'lastName' },
533+
},
534+
],
535+
},
386536
]),
387537
});

0 commit comments

Comments
 (0)