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

[new] add static-property-placement rule #2193

Merged
merged 1 commit into from Apr 12, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Expand Up @@ -142,6 +142,7 @@ Enable the rules that you would like to use.
* [react/sort-comp](docs/rules/sort-comp.md): Enforce component methods order (fixable)
* [react/sort-prop-types](docs/rules/sort-prop-types.md): Enforce propTypes declarations alphabetical sorting
* [react/state-in-constructor](docs/rules/state-in-constructor.md): Enforce the state initialization style to be either in a constructor or with a class property
* [react/static-property-placement](docs/rules/static-property-placement.md): Defines where React component static properties should be positioned.
* [react/style-prop-object](docs/rules/style-prop-object.md): Enforce style prop value being an object
* [react/void-dom-elements-no-children](docs/rules/void-dom-elements-no-children.md): Prevent void DOM elements (e.g. `<img />`, `<br />`) from receiving children

Expand Down
185 changes: 185 additions & 0 deletions docs/rules/static-property-placement.md
@@ -0,0 +1,185 @@
# Enforces where React component static properties should be positioned. (static-property-placement)

This rule allows you to enforce where `childContextTypes`, `contextTypes`, `contextType`, `defaultProps`, `displayName`,
and `propTypes` are declared in an ES6 class.


## Rule Details

By default, this rule will check for and warn about declaring any of the above properties outside of the class body.

There are three key options are `static public field`, `static getter`, and `property assignment`.

### When `static public field` is enabled (default):

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

```js
class MyComponent extends React.Component {
static get childContextTypes() { /*...*/ }
static get contextTypes() { /*...*/ }
static get contextType() { /*...*/ }
static get displayName() { /*...*/ }
static get defaultProps() { /*...*/ }
static get propTypes() { /*...*/ }
}
```

```js
class MyComponent extends React.Component { /*...*/ }
MyComponent.childContextTypes = { /*...*/ };
MyComponent.contextTypes = { /*...*/ };
MyComponent.contextType = { /*...*/ };
MyComponent.displayName = "Hello";
MyComponent.defaultProps = { /*...*/ };
MyComponent.propTypes = { /*...*/ };
```

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

```js
class MyComponent extends React.Component {
static childContextTypes = { /*...*/ };
static contextTypes = { /*...*/ };
static contextType = { /*...*/ };
static displayName = "Hello";
static defaultProps = { /*...*/ };
static propTypes = { /*...*/ };
}
```

### When `static getter` is enabled:

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

```js
class MyComponent extends React.Component {
static childContextTypes = { /*...*/ };
static contextTypes = { /*...*/ };
static contextType = { /*...*/ };
static displayName = "Hello";
static defaultProps = { /*...*/ };
static propTypes = { /*...*/ };
}
```

```js
class MyComponent extends React.Component { /*...*/ }
MyComponent.childContextTypes = { /*...*/ };
MyComponent.contextTypes = { /*...*/ };
MyComponent.contextType = { /*...*/ };
MyComponent.displayName = "Hello";
MyComponent.defaultProps = { /*...*/ };
MyComponent.propTypes = { /*...*/ };
```

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

```js
class MyComponent extends React.Component {
static get childContextTypes() { /*...*/ }
static get contextTypes() { /*...*/ }
static get contextType() { /*...*/ }
static get displayName() { /*...*/ }
static get defaultProps() { /*...*/ }
static get propTypes() { /*...*/ }
}
```

### When `property assignment` is enabled:

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

```js
class MyComponent extends React.Component {
static childContextTypes = { /*...*/ };
static contextTypes = { /*...*/ };
static contextType = { /*...*/ };
static displayName = "Hello";
static defaultProps = { /*...*/ };
static propTypes = { /*...*/ };
}
```

```js
class MyComponent extends React.Component {
static get childContextTypes() { /*...*/ }
static get contextTypes() { /*...*/ }
static get contextType() { /*...*/ }
static get displayName() { /*...*/ }
static get defaultProps() { /*...*/ }
static get propTypes() { /*...*/ }
}
```

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

```js
class MyComponent extends React.Component { /*...*/ }
MyComponent.childContextTypes = { /*...*/ };
MyComponent.contextTypes = { /*...*/ };
MyComponent.contextType = { /*...*/ };
MyComponent.displayName = "Hello";
MyComponent.defaultProps = { /*...*/ };
MyComponent.propTypes = { /*...*/ };
```

### Options

```
...
"react/static-property-placement": [<enabled>] // `static public field` enabled
...
```

or alternatively:

```
...
"react/static-property-placement": [<enabled>, <string>]
...
```

or alternatively:

```
...
"react/static-property-placement": [<enabled>, <string>, {
childContextTypes: <string>,
contextTypes: <string>,
contextType: <string>,
defaultProps: <string>,
displayName: <string>,
propTypes: <string>,
}]
...
```
The `<string>` value must be one these options:
* `static public field`
* `static getter`
* `property assignment`

The `options` schema defined above allows you to specify different rules for the different property fields available.

##### Example configuration:
_This is only an example, we do not recommend this as a configuration._
```
...
"react/static-property-placement": ["warn", "property assignment", {
childContextTypes: "static getter",
contextTypes: "static public field",
contextType: "static public field",
displayName: "static public field",
}]
...
```

Based on the above configuration:
* `defaultProps` and `propTypes` will both enforce the `property assignment` rule.
* `childContextTypes` will enforce the `static getter` rule.
* `contextTypes`, `contextType`, and `displayName` will enforce the `static public field` rule.

## When Not To Use It

If you have no placement preference for React's static class properties.

1 change: 1 addition & 0 deletions index.js
Expand Up @@ -82,6 +82,7 @@ const allRules = {
'sort-comp': require('./lib/rules/sort-comp'),
'sort-prop-types': require('./lib/rules/sort-prop-types'),
'state-in-constructor': require('./lib/rules/state-in-constructor'),
'static-property-placement': require('./lib/rules/static-property-placement'),
'style-prop-object': require('./lib/rules/style-prop-object'),
'void-dom-elements-no-children': require('./lib/rules/void-dom-elements-no-children')
};
Expand Down
27 changes: 5 additions & 22 deletions lib/rules/display-name.js
Expand Up @@ -7,6 +7,7 @@
const Components = require('../util/Components');
const astUtil = require('../util/ast');
const docsUrl = require('../util/docsUrl');
const propsUtil = require('../util/props');

// ------------------------------------------------------------------------------
// Rule Definition
Expand Down Expand Up @@ -38,24 +39,6 @@ module.exports = {

const MISSING_MESSAGE = 'Component definition is missing display name';

/**
* Checks if we are declaring a display name
* @param {ASTNode} node The AST node being checked.
* @returns {Boolean} True if we are declaring a display name, false if not.
*/
function isDisplayNameDeclaration(node) {
switch (node.type) {
case 'ClassProperty':
return node.key && node.key.name === 'displayName';
case 'Identifier':
return node.name === 'displayName';
case 'Literal':
return node.value === 'displayName';
default:
return false;
}
}

/**
* Mark a prop type as declared
* @param {ASTNode} node The AST node being checked.
Expand Down Expand Up @@ -139,14 +122,14 @@ module.exports = {
return {

ClassProperty: function(node) {
if (!isDisplayNameDeclaration(node)) {
if (!propsUtil.isDisplayNameDeclaration(node)) {
return;
}
markDisplayNameAsDeclared(node);
},

MemberExpression: function(node) {
if (!isDisplayNameDeclaration(node.property)) {
if (!propsUtil.isDisplayNameDeclaration(node.property)) {
return;
}
const component = utils.getRelatedComponent(node);
Expand Down Expand Up @@ -184,7 +167,7 @@ module.exports = {
},

MethodDefinition: function(node) {
if (!isDisplayNameDeclaration(node.key)) {
if (!propsUtil.isDisplayNameDeclaration(node.key)) {
return;
}
markDisplayNameAsDeclared(node);
Expand All @@ -208,7 +191,7 @@ module.exports = {
if (ignoreTranspilerName || !hasTranspilerName(node)) {
// Search for the displayName declaration
node.properties.forEach(property => {
if (!property.key || !isDisplayNameDeclaration(property.key)) {
if (!property.key || !propsUtil.isDisplayNameDeclaration(property.key)) {
return;
}
markDisplayNameAsDeclared(node);
Expand Down