Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ Finally, enable all of the rules that you would like to use. Use [our preset](#
* [react/no-set-state](docs/rules/no-set-state.md): Prevent usage of `setState`
* [react/no-typos](docs/rules/no-typos.md): Prevent common casing typos
* [react/no-string-refs](docs/rules/no-string-refs.md): Prevent using string references in `ref` attribute.
* [react/no-this-in-sfc](docs/rules/no-this-in-sfc.md): Prevent using `this` in stateless functional components
* [react/no-unescaped-entities](docs/rules/no-unescaped-entities.md): Prevent invalid characters from appearing in markup
* [react/no-unknown-property](docs/rules/no-unknown-property.md): Prevent usage of unknown DOM property (fixable)
* [react/no-unused-prop-types](docs/rules/no-unused-prop-types.md): Prevent definitions of unused prop types
Expand Down
136 changes: 136 additions & 0 deletions docs/rules/no-this-in-sfc.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
# Prevent `this` from being used in stateless functional components (react/no-this-in-sfc)

When using a stateless functional component (SFC), props/context aren't accessed in the same way as a class component or the `create-react-class` format. Both props and context are passed as separate arguments to the component instead. Also, as the name suggests, a stateless component does not have state on `this.state`.

Attempting to access properties on `this` can be a potential error if someone is unaware of the differences when writing a SFC or missed when converting a class component to a SFC.


## Rule Details

The following patterns are considered warnings:

```jsx
function Foo(props) {
return (
<div>{this.props.bar}</div>
);
}
```

```jsx
function Foo(props) {
const { bar } = this.props;
return (
<div>{bar}</div>
);
}
```

```jsx
function Foo(props, context) {
return (
<div>
{this.context.foo ? this.props.bar : ''}
</div>
);
}
```

```jsx
function Foo(props, context) {
const { foo } = this.context;
const { bar } = this.props;
return (
<div>
{foo ? bar : ''}
</div>
);
}
```

```jsx
function Foo(props) {
if (this.state.loading) {
return <Loader />;
}
return (
<div>
{this.props.bar}
</div>
);
}
```

```jsx
function Foo(props) {
const { loading } = this.state;
const { bar } = this.props;
if (loading) {
return <Loader />;
}
return (
<div>
{bar}
</div>
);
}
```

The following patterns are **not** considered warnings:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's also add non-warning patterns (and test cases) for destructured props/context


```jsx
function Foo(props) {
return (
<div>{props.bar}</div>
);
}
```

```jsx
function Foo(props) {
const { bar } = props;
return (
<div>{bar}</div>
);
}
```

```jsx
function Foo({ bar }) {
return (
<div>{bar}</div>
);
}
```

```jsx
function Foo(props, context) {
return (
<div>
{context.foo ? props.bar : ''}
</div>
);
}
```

```jsx
function Foo(props, context) {
const { foo } = context;
const { bar } = props;
return (
<div>
{foo ? bar : ''}
</div>
);
}
```

```jsx
function Foo({ bar }, { foo }) {
return (
<div>
{foo ? bar : ''}
</div>
);
}
```
1 change: 1 addition & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ const allRules = {
'no-string-refs': require('./lib/rules/no-string-refs'),
'no-redundant-should-component-update': require('./lib/rules/no-redundant-should-component-update'),
'no-render-return-value': require('./lib/rules/no-render-return-value'),
'no-this-in-sfc': require('./lib/rules/no-this-in-sfc'),
'no-typos': require('./lib/rules/no-typos'),
'no-unescaped-entities': require('./lib/rules/no-unescaped-entities'),
'no-unknown-property': require('./lib/rules/no-unknown-property'),
Expand Down
42 changes: 42 additions & 0 deletions lib/rules/no-this-in-sfc.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/**
* @fileoverview Report "this" being used in stateless functional components.
*/
'use strict';

const Components = require('../util/Components');

// ------------------------------------------------------------------------------
// Constants
// ------------------------------------------------------------------------------

const ERROR_MESSAGE = 'Stateless functional components should not use this';

// ------------------------------------------------------------------------------
// Rule Definition
// ------------------------------------------------------------------------------

module.exports = {
meta: {
docs: {
description: 'Report "this" being used in stateless components',
category: 'Possible Errors',
recommended: false
},
schema: []
},

create: Components.detect((context, components, utils) => ({
MemberExpression(node) {
const component = components.get(utils.getParentStatelessComponent());
if (!component) {
return;
}
if (node.object.type === 'ThisExpression') {
context.report({
node: node,
message: ERROR_MESSAGE
});
}
}
}))
};
68 changes: 56 additions & 12 deletions lib/util/Components.js
Original file line number Diff line number Diff line change
Expand Up @@ -303,14 +303,7 @@ function componentRule(rule, context) {
return calledOnReact;
},

/**
* Check if the node is returning JSX
*
* @param {ASTNode} ASTnode The AST node being checked
* @param {Boolean} strict If true, in a ternary condition the node must return JSX in both cases
* @returns {Boolean} True if the node is returning JSX, false if not
*/
isReturningJSX: function(ASTnode, strict) {
getReturnPropertyAndNode(ASTnode) {
let property;
let node = ASTnode;
switch (node.type) {
Expand All @@ -319,14 +312,36 @@ function componentRule(rule, context) {
break;
case 'ArrowFunctionExpression':
property = 'body';
if (node[property] && node[property].type === 'BlockStatement') {
node = utils.findReturnStatement(node);
property = 'argument';
}
break;
default:
node = utils.findReturnStatement(node);
if (!node) {
return false;
}
property = 'argument';
}
return {
node: node,
property: property
};
},

/**
* Check if the node is returning JSX
*
* @param {ASTNode} ASTnode The AST node being checked
* @param {Boolean} strict If true, in a ternary condition the node must return JSX in both cases
* @returns {Boolean} True if the node is returning JSX, false if not
*/
isReturningJSX: function(ASTnode, strict) {
const nodeAndProperty = utils.getReturnPropertyAndNode(ASTnode);
const node = nodeAndProperty.node;
const property = nodeAndProperty.property;

if (!node) {
return false;
}

const returnsConditionalJSXConsequent =
node[property] &&
Expand Down Expand Up @@ -356,6 +371,35 @@ function componentRule(rule, context) {
);
},

/**
* Check if the node is returning null
*
* @param {ASTNode} ASTnode The AST node being checked
* @returns {Boolean} True if the node is returning null, false if not
*/
isReturningNull(ASTnode) {
const nodeAndProperty = utils.getReturnPropertyAndNode(ASTnode);
const property = nodeAndProperty.property;
const node = nodeAndProperty.node;

if (!node) {
return false;
}

return node[property] && node[property].value === null;
},

/**
* Check if the node is returning JSX or null
*
* @param {ASTNode} ASTnode The AST node being checked
* @param {Boolean} strict If true, in a ternary condition the node must return JSX in both cases
* @returns {Boolean} True if the node is returning JSX or null, false if not
*/
isReturningJSXOrNull(ASTNode, strict) {
return utils.isReturningJSX(ASTNode, strict) || utils.isReturningNull(ASTNode);
},

/**
* Find a return statment in the current node
*
Expand Down Expand Up @@ -430,7 +474,7 @@ function componentRule(rule, context) {
return null;
}
// Return the node if it is a function that is not a class method and is not inside a JSX Element
if (isFunction && !isMethod && !isJSXExpressionContainer) {
if (isFunction && !isMethod && !isJSXExpressionContainer && utils.isReturningJSXOrNull(node)) {
return node;
}
scope = scope.upper;
Expand Down
Loading