Skip to content
This repository has been archived by the owner on Aug 18, 2020. It is now read-only.

Commit

Permalink
clean up
Browse files Browse the repository at this point in the history
  • Loading branch information
cartogram committed Aug 20, 2018
1 parent 93bd777 commit 2bd5e5a
Show file tree
Hide file tree
Showing 8 changed files with 126 additions and 39 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
# Changelog

## Unreleased
## Unreleased

### Fixed
* `plugin:shopify/flow` now disables rules checked by Flow's static analyzer. ([#135](https://github.com/Shopify/eslint-plugin-shopify/pull/135))
* `plugin:shopify/prettier` and `plugin:shopify/typescript-prettier` defer missing semicolon rules to a project´s `.prettierrc`. ([#135](https://github.com/Shopify/eslint-plugin-shopify/pull/135))

### Added
* `shopify/prefer-singular-enums` ([#132](https://github.com/Shopify/eslint-plugin-shopify/pull/132))
* `shopify/react-no-multiple-render-methods` ([#134](https://github.com/Shopify/eslint-plugin-shopify/pull/134))

## [23.1.0] - 2018-08-02

Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ This plugin provides the following custom rules, which are included as appropria
- [prefer-singular-enums](docs/rules/prefer-singular-enums.md): Prefer TypeScript enums be singular.
- [prefer-twine](docs/rules/prefer-twine.md): Prefer Twine over Bindings as the name for twine imports.
- [react-initialize-state](docs/rules/react-initialize-state.md): Require that React component state be initialized when it has a non-empty type.
- [react-no-multiple-render-methods](docs/rules/react-no-multiple-render-methods.md): Disallow multiple render methods in React component classes.
- [react-prefer-private-members](docs/rules/react-prefer-private-members.md): Prefer all non-React-specific members be marked private in React class components.
- [react-type-state](docs/rules/react-type-state.md): Require that React component state be typed in TypeScript.
- [restrict-full-import](docs/rules/restrict-full-import.md): Prevent importing the entirety of a package.
Expand Down
49 changes: 49 additions & 0 deletions docs/rules/react-no-multiple-render-methods.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# Disallow multiple render methods in React components. (react-no-multiple-render-methods)

As a React component grows to render many elements, it is tempting to split the render method into multiple “sub-render” methods. While this may seem like an improvement in readability, the component's state, props, and class methods are still shared, making it difficult to identify which are used by each additional render method. The entire class becomes objectively more complicated, and it would be more effective to break those additional elements into entirely new components instead.

## Rule Details

This rule disallows any `renderFoo` methods on a class.

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


```ts
class MyComponent extends React.Component {
renderFoo() {
return (
<div>
{this.state.bar}
</div>
)
}
render() {
return (
<div>
{this.renderFoo()}
</div>
)
}
}
```

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

```ts
import Foo from './components/Foo';

class MyComponent extends React.Component {
render() {
return (
<div>
<Foo bar={this.state.bar} />
</div>
)
}
}
```

## When Not To Use It

If you do not want to restrict render methods on React class components, you can safely disable this rule.
1 change: 1 addition & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ module.exports = {
'prefer-singular-enums': require('./lib/rules/prefer-singular-enums'),
'prefer-twine': require('./lib/rules/prefer-twine'),
'react-initialize-state': require('./lib/rules/react-initialize-state'),
'react-no-multiple-render-methods': require('./lib/rules/react-no-multiple-render-methods'),
'react-prefer-private-members': require('./lib/rules/react-prefer-private-members'),
'react-type-state': require('./lib/rules/react-type-state'),
'restrict-full-import': require('./lib/rules/restrict-full-import'),
Expand Down
2 changes: 2 additions & 0 deletions lib/config/rules/shopify.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ module.exports = {
'shopify/prefer-twine': 'error',
// Require that React component state be initialized when it has a non-empty type.
'shopify/react-initialize-state': 'off',
// Disallow multiple render methods in React component classes.
'shopify/react-no-multiple-render-methods': 'off',
// Prefer all non-React-specific members be marked private in React class components.
'shopify/react-prefer-private-members': 'off',
// Require that React component state be typed in TypeScript.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,21 @@
const Components = require('eslint-plugin-react/lib/util/Components');

const message = [
'Don’t use multiple render methods in a single component;',
'they generally make your component harder to read.',
'Instead break {{name}} out into its own component',
'and render it inside this one.',
].join(' ');

module.exports = {
meta: {
docs: {
description: 'Disallow renderX methods within React component classes',
description:
'Disallow multiple render methods in React component classes',
category: 'Best Practices',
recommended: true,
uri:
'https://github.com/Shopify/eslint-plugin-shopify/blob/master/docs/rules/react-no-render-methods.md',
'https://github.com/Shopify/eslint-plugin-shopify/blob/master/docs/rules/react-no-multiple-render-methods.md',
},
},

Expand All @@ -21,7 +29,7 @@ module.exports = {

context.report({
node,
message: `No renderX methods. '{{name}}' should be a seperate component.`,
message,
data: {name},
});
}
Expand All @@ -30,6 +38,7 @@ module.exports = {
ClassDeclaration(node) {
isES6Component = utils.isES6Component(node);
},

MethodDefinition(node) {
if (!isES6Component || isRenderMethod(node)) {
return;
Expand Down
59 changes: 59 additions & 0 deletions tests/lib/rules/react-no-multiple-render-methods.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
const {RuleTester} = require('eslint');
const rule = require('../../../lib/rules/react-no-multiple-render-methods');

const ruleTester = new RuleTester();

require('babel-eslint');

const babelParser = 'babel-eslint';

function error(memberName) {
return {
type: 'MethodDefinition',
message: `Don’t use multiple render methods in a single component; they generally make your component harder to read. Instead break ${memberName} out into its own component and render it inside this one.`,
};
}

ruleTester.run('react-no-multiple-render-methods', rule, {
valid: [
{
code: `class Button extends React.Component {
render() {}
}`,
parser: babelParser,
},
{
code: `class Button extends React.Component {
otherMethod() {}
render() {}
}`,
parser: babelParser,
},
],
invalid: [
{
code: `class Button extends React.Component {
renderFoo() {}
}`,
parser: babelParser,
errors: [error('renderFoo')],
},
{
code: `class Button extends React.Component {
renderFoo() {}
render() {}
}`,
parser: babelParser,
errors: [error('renderFoo')],
},
{
code: `class Button extends React.Component {
renderFoo() {}
renderBar() {}
render() {}
}`,
parser: babelParser,
errors: [error('renderFoo'), error('renderBar')],
},
],
});
35 changes: 0 additions & 35 deletions tests/lib/rules/react-no-render-methods.js

This file was deleted.

0 comments on commit 2bd5e5a

Please sign in to comment.