This repository has been archived by the owner on Aug 18, 2020. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 18
Add react-prefer-private-members
rule
#95
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
# Disallow public members within React component classes (react-prefer-private-members) | ||
|
||
This rule enforces all non-React-specific members be marked private in React class components. | ||
|
||
## Rule Details | ||
|
||
When a member of class is marked private, it cannot be accessed from outside of its containing class. This restriction is prefered when writing React components, where encapsulation is almost always desireable. | ||
|
||
The following patterns are considered warnings: | ||
|
||
```ts | ||
class MyComponent extends React.Component<Props, State> { | ||
publicMethod() {} | ||
alsoPublic: string; | ||
} | ||
``` | ||
|
||
The following patterns are not warnings: | ||
```ts | ||
class MyComponent extends React.Component<Props, State> { | ||
private publicMethod() {} | ||
private alsoPublic: string; | ||
} | ||
``` | ||
|
||
The Lifecycle methods and static properties that are part of the React API are not required to be private for this rule. | ||
|
||
```ts | ||
class MyComponent extends React.Component<Props, State> { | ||
private publicMethod() {} | ||
private alsoPublic: string; | ||
static propTypes = {} | ||
static defaultProps = {} | ||
static childContextTypes = {} | ||
static contextTypes = {} | ||
getDerivedStateFromProps() {} | ||
componentWillMount() {} | ||
UNSAFE_componentWillMount() {} | ||
componentDidMount() {} | ||
componentWillReceiveProps() {} | ||
UNSAFE_componentWillReceiveProps() {} | ||
shouldComponentUpdate() {} | ||
componentWillUpdate() {} | ||
UNSAFE_componentWillUpdate() {} | ||
getSnapshotBeforeUpdate() {} | ||
componentDidUpdate() {} | ||
componentDidCatch() {} | ||
componentWillUnmount() {} | ||
render() {} | ||
} | ||
``` | ||
|
||
Exposing subcomponents as public static members is not considered a warning for this rule. | ||
|
||
```ts | ||
class MyComponent extends React.Component<Props, State> { | ||
static MySubComponent = MySubComponent; | ||
render() {} | ||
} | ||
``` | ||
|
||
## When Not To Use It | ||
|
||
If you do not want to restrict public members on React class components, you can safely disable this rule. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,93 @@ | ||
const pascalCase = require('pascal-case'); | ||
const Components = require('eslint-plugin-react/lib/util/Components'); | ||
|
||
module.exports = { | ||
meta: { | ||
docs: { | ||
description: 'Disallow public members within React component classes', | ||
category: 'Best Practices', | ||
recommended: true, | ||
uri: | ||
'https://github.com/Shopify/eslint-plugin-shopify/blob/master/docs/rules/react-prefer-private-members.md', | ||
}, | ||
}, | ||
|
||
create: Components.detect((context, components, utils) => { | ||
let isES6Component = false; | ||
let componentName = null; | ||
|
||
function report({node, componentName: classComponent}) { | ||
const { | ||
key: {name}, | ||
} = node; | ||
|
||
context.report({ | ||
node, | ||
message: `'{{name}}' should be a private member of '{{classComponent}}'.`, | ||
data: {name, classComponent}, | ||
}); | ||
} | ||
|
||
return { | ||
ClassDeclaration(node) { | ||
isES6Component = utils.isES6Component(node); | ||
componentName = node.id.name; | ||
}, | ||
ClassProperty(node) { | ||
if (!isES6Component || isValid(node)) { | ||
return; | ||
} | ||
|
||
report({node, componentName}); | ||
}, | ||
MethodDefinition(node) { | ||
if (!isES6Component || isValid(node)) { | ||
return; | ||
} | ||
|
||
report({node, componentName}); | ||
}, | ||
}; | ||
}), | ||
}; | ||
|
||
function isValid(node) { | ||
return ( | ||
node.accessibility === 'private' || | ||
isReactLifeCycleMethod(node) || | ||
isReactStaticProperty(node) || | ||
isCompoundComponentMember(node) | ||
); | ||
} | ||
|
||
function isReactLifeCycleMethod({key: {name}}) { | ||
return [ | ||
'getDerivedStateFromProps', | ||
'componentWillMount', | ||
'UNSAFE_componentWillMount', | ||
'componentDidMount', | ||
'componentWillReceiveProps', | ||
'UNSAFE_componentWillReceiveProps', | ||
'shouldComponentUpdate', | ||
'componentWillUpdate', | ||
'UNSAFE_componentWillUpdate', | ||
'getSnapshotBeforeUpdate', | ||
'componentDidUpdate', | ||
'componentDidCatch', | ||
'componentWillUnmount', | ||
'render', | ||
].some((method) => method === name); | ||
} | ||
|
||
function isReactStaticProperty({key: {name}}) { | ||
return [ | ||
'propTypes', | ||
'contextTypes', | ||
'childContextTypes', | ||
'defaultProps', | ||
].some((method) => method === name); | ||
} | ||
|
||
function isCompoundComponentMember({key: {name}}) { | ||
return name === pascalCase(name); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,139 @@ | ||
const {RuleTester} = require('eslint'); | ||
const rule = require('../../../lib/rules/react-prefer-private-members'); | ||
|
||
const ruleTester = new RuleTester(); | ||
|
||
require('babel-eslint'); | ||
require('typescript-eslint-parser'); | ||
|
||
const babelParser = 'babel-eslint'; | ||
const typeScriptParser = 'typescript-eslint-parser'; | ||
|
||
function makeError({type = 'ClassProperty', memberName, componentName}) { | ||
return { | ||
type, | ||
message: `'${memberName}' should be a private member of '${componentName}'.`, | ||
}; | ||
} | ||
|
||
ruleTester.run('react-prefer-private-members', rule, { | ||
valid: [ | ||
{ | ||
code: `class Button extends React.Component { | ||
private member = true; | ||
componentDidMount() {} | ||
}`, | ||
parser: typeScriptParser, | ||
}, | ||
{ | ||
code: `class Button extends Klass { | ||
publicMember = true | ||
publicMethod() {} | ||
}`, | ||
parser: babelParser, | ||
}, | ||
{ | ||
code: 'class Button extends React.Component {}', | ||
parser: babelParser, | ||
}, | ||
{ | ||
code: `class KitchenSink extends React.Component { | ||
static propTypes = {} | ||
static defaultProps = {} | ||
static childContextTypes = {} | ||
static contextTypes = {} | ||
getDerivedStateFromProps() {} | ||
componentWillMount() {} | ||
UNSAFE_componentWillMount() {} | ||
componentDidMount() {} | ||
componentWillReceiveProps() {} | ||
UNSAFE_componentWillReceiveProps() {} | ||
shouldComponentUpdate() {} | ||
componentWillUpdate() {} | ||
UNSAFE_componentWillUpdate() {} | ||
getSnapshotBeforeUpdate() {} | ||
componentDidUpdate() {} | ||
componentDidCatch() {} | ||
componentWillUnmount() {} | ||
render() {} | ||
}`, | ||
parser: babelParser, | ||
}, | ||
{ | ||
code: `class CompoundComponent extends React.Component { | ||
static propTypes = {} | ||
static Item = Item | ||
static AnotherItem = AnotherItem | ||
render() {} | ||
}`, | ||
parser: babelParser, | ||
}, | ||
], | ||
invalid: [ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Need more tests than this (missing for private members, static members, etc |
||
{ | ||
code: `class Button extends React.Component { | ||
publicMember = true; | ||
componentDidMount() {} | ||
}`, | ||
parser: babelParser, | ||
errors: [ | ||
makeError({memberName: 'publicMember', componentName: 'Button'}), | ||
], | ||
}, | ||
{ | ||
code: `class Button extends React.Component { | ||
static Valid = Valid; | ||
static inValid = inValid; | ||
render() {} | ||
}`, | ||
parser: babelParser, | ||
errors: [makeError({memberName: 'inValid', componentName: 'Button'})], | ||
}, | ||
{ | ||
code: `class Button extends React.Component { | ||
private validMember: string; | ||
private alsoValidMember() {}; | ||
inValid: string; | ||
alsoInvalid() {} | ||
render() {} | ||
}`, | ||
parser: typeScriptParser, | ||
errors: [ | ||
makeError({memberName: 'inValid', componentName: 'Button'}), | ||
makeError({ | ||
type: 'MethodDefinition', | ||
memberName: 'alsoInvalid', | ||
componentName: 'Button', | ||
}), | ||
], | ||
}, | ||
{ | ||
code: `class Button extends React.Component { | ||
publicMethod() {} | ||
componentDidMount() {} | ||
}`, | ||
parser: babelParser, | ||
errors: [ | ||
makeError({ | ||
type: 'MethodDefinition', | ||
memberName: 'publicMethod', | ||
componentName: 'Button', | ||
}), | ||
], | ||
}, | ||
{ | ||
code: `class PureButton extends React.PureComponent { | ||
publicMethod() {} | ||
componentDidMount() {} | ||
}`, | ||
parser: babelParser, | ||
errors: [ | ||
makeError({ | ||
type: 'MethodDefinition', | ||
memberName: 'publicMethod', | ||
componentName: 'PureButton', | ||
}), | ||
], | ||
}, | ||
], | ||
}); |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you probably want to allow all static properties, or at least those that are pascal-case, to allow for our (pretty common) pattern of exposing subcomponent as public static members.