Skip to content

Commit 2b4c10c

Browse files
authored
fix(eslint-plugin): reinstating require-named-effects (#86)
* revert(eslint-plugin): "fix: remove useless required line in index (#84)" This reverts commit 4c81100. * revert(eslint-plugin): "revert(eslint-plugin): delete custom rules (#81)" This reverts commit e6fcf3b. * chore(eslint-plugin): delete CI test check * fix(eslint-plugin): remove auto-generating doc command * doc(eslint-plugin): delete 5 custom rules * BREAKING CHANGE(eslint-plugin): delete custom rules * fix(eslint-plugin): delete example of custom rules * doc(eslint-plugin): removing 5 custom rules doc * chore(eslint-plugin): enabling custom rule test in example-app * fix(eslint-plugin): adding a todo for no-floating-promises bugfix * fix(eslint-plugin): absolute path rule * chore(eslint-plugin): adding notion ticket in todo * chore(eslint-plugin): re-adding scripts for custom rules * fix(eslint-plugin): auto generated doc * chroe(eslint-plugin): re-adding CI test
1 parent 010118d commit 2b4c10c

10 files changed

Lines changed: 223 additions & 3 deletions

File tree

.github/workflows/quality-eslint.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,3 +38,6 @@ jobs:
3838

3939
- name: Linter
4040
run: yarn lint
41+
42+
- name: Tests
43+
run: yarn test

example-app/.eslintrc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,6 @@
66
"files": ["*.test.tsx"],
77
"extends": "plugin:@bam.tech/tests"
88
}
9-
]
9+
],
10+
"rules": { "@bam.tech/require-named-effect": "error" }
1011
}
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// Save without formatting: [⌘ + K] > [S]
2+
3+
// This should trigger an error breaking eslint-plugin-bam-custom-rules:
4+
// bam-custom-rules/require-named-effect
5+
6+
import { useEffect } from "react";
7+
8+
const MyComponent = () => {
9+
const doAThing = (t: number) => t;
10+
11+
useEffect(() => {
12+
const abc = 1;
13+
doAThing(abc);
14+
}, []);
15+
16+
return <></>;
17+
};
18+
19+
export default MyComponent;

packages/eslint-plugin/README.md

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,32 @@ If you use **npm >= 5** you can automatically install them by running:
9696
npx install-peerdeps @bam.tech/eslint-config -D
9797
```
9898

99+
## Eslint rules
100+
101+
This plugin exports some custom rules that you can optionally use in your project:
102+
103+
<!-- begin auto-generated rules list -->
104+
105+
| Name | Description |
106+
| :------------------------------------------------------------------------------------------------------------------------------------------------ | :----------------------------------------------------- |
107+
| [require-named-effect](https://github.com/bamlab/react-native-project-config/blob/main/packages/eslint-plugin/docs/rules/require-named-effect.md) | Enforces the use of named functions inside a useEffect |
108+
109+
<!-- end auto-generated rules list -->
110+
111+
To use a rule, just declare it in your `.eslintrc`:
112+
113+
```json
114+
// .eslintrc
115+
{
116+
"plugins": ["@bam.tech"],
117+
"rules": {
118+
"@bam.tech/require-named-effect": "error"
119+
}
120+
}
121+
```
122+
123+
> _Tip: if your config is already extended from a `@bam.tech` config, you don't need to declare the plugin._
124+
99125
## How to customize?
100126

101127
You can still customize your ESLint config by adding other configurations, plugins and rules to your `.eslintrc` config file.
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
# Enforces the use of named functions inside a useEffect (`@bam.tech/require-named-effect`)
2+
3+
<!-- end auto-generated rule header -->
4+
5+
Force to use named functions inside a useEffect instead of lambda functions.
6+
7+
## Rule Details
8+
9+
Examples of **incorrect** code for this rule:
10+
11+
```jsx
12+
useEffect(() => {}, []);
13+
```
14+
15+
```jsx
16+
useEffect(() => {
17+
const t = 1;
18+
disallowTwoThings(t);
19+
}, []);
20+
```
21+
22+
Examples of **correct** code for this rule:
23+
24+
```jsx
25+
useEffect(function namedFunction() {}, []);
26+
```
27+
28+
```jsx
29+
useEffect(theNameOfAFunction(), []);
30+
```
31+
32+
```jsx
33+
useEffect(() => theNameOfAFunction(), []);
34+
```
35+
36+
```jsx
37+
useEffect(() => {
38+
theOnlyChildIsAFunctionCall();
39+
}, []);
40+
```

packages/eslint-plugin/lib/configs/recommended.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ module.exports = defineConfig({
4444
"@typescript-eslint/no-unused-vars": "off",
4545
"unused-imports/no-unused-imports": "error",
4646
"unused-imports/no-unused-vars": "error",
47-
"@typescript-eslint/no-floating-promises": "error",
47+
// "@typescript-eslint/no-floating-promises": "error", //TODO: A réactiver après le fix de @thomasrebam https://www.notion.so/m33/ETQU-sur-le-plugin-eslint-la-r-gle-no-floating-promises-fonctionne-949b4beca2b34704a87c41faa982f540
4848
},
4949
env: {
5050
node: true,

packages/eslint-plugin/lib/index.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,5 @@ const requireIndex = require("requireindex");
44

55
module.exports = {
66
configs: requireIndex(__dirname + "/configs"),
7+
rules: requireIndex(__dirname + "/rules"),
78
};
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
/**
2+
* @fileoverview Force the use of named functions inside a useEffect
3+
* @author Cyril Bonaccini
4+
*/
5+
"use strict";
6+
7+
//------------------------------------------------------------------------------
8+
// Rule Definition
9+
//------------------------------------------------------------------------------
10+
11+
/** @type {import('eslint').Rule.RuleModule} */
12+
module.exports = {
13+
meta: {
14+
type: "suggestion", // `problem`, `suggestion`, or `layout`
15+
docs: {
16+
description: "Enforces the use of named functions inside a useEffect",
17+
recommended: false,
18+
url: "https://github.com/bamlab/react-native-project-config/tree/main/packages/eslint-plugin/docs/rules/require-named-effect.md", // URL to the documentation page for this rule
19+
},
20+
fixable: null, // Or `code` or `whitespace`
21+
schema: [], // Add a schema if the rule has options,
22+
messages: {
23+
useNamedFunction: "Complex effects must be a named function.",
24+
},
25+
},
26+
27+
create(context) {
28+
//----------------------------------------------------------------------
29+
// Helpers
30+
//----------------------------------------------------------------------
31+
32+
const isUseEffect = (node) => node.callee.name === "useEffect";
33+
34+
const argumentIsArrowFunction = (node) =>
35+
node.arguments[0].type === "ArrowFunctionExpression";
36+
37+
const effectBodyIsSingleFunction = (node) => {
38+
const { body } = node.arguments[0];
39+
40+
// It's a single unwrapped function call:
41+
// `useEffect(() => theNameOfAFunction(), []);`
42+
if (body.type === "CallExpression") {
43+
return true;
44+
}
45+
46+
// There's a function body, but it just calls another function:
47+
// `useEffect(() => {
48+
// theOnlyChildIsAFunctionCall();
49+
// }, []);`
50+
if (
51+
body.body.length &&
52+
body.body.length === 1 &&
53+
body.body[0] &&
54+
body.body[0].expression &&
55+
body.body[0].expression.type === "CallExpression"
56+
) {
57+
return true;
58+
}
59+
60+
return false;
61+
};
62+
63+
const fail = (report, node) =>
64+
report({ node, messageId: "useNamedFunction" });
65+
66+
//----------------------------------------------------------------------
67+
// Public
68+
//----------------------------------------------------------------------
69+
70+
return {
71+
CallExpression(node) {
72+
if (
73+
isUseEffect(node) &&
74+
argumentIsArrowFunction(node) &&
75+
!effectBodyIsSingleFunction(node)
76+
) {
77+
fail(context.report, node);
78+
}
79+
},
80+
};
81+
},
82+
};

packages/eslint-plugin/package.json

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,10 @@
1818
"license": "MIT",
1919
"scripts": {
2020
"lint": "npm-run-all \"lint:*\"",
21-
"lint:js": "eslint ."
21+
"lint:js": "eslint .",
22+
"lint:eslint-docs": "npm-run-all \"update:eslint-docs -- --check\"",
23+
"test": "mocha tests --recursive",
24+
"update:eslint-docs": "eslint-doc-generator"
2225
},
2326
"peerDependencies": {
2427
"@typescript-eslint/eslint-plugin": "^5.61.0",
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
/**
2+
* @fileoverview Force the use of named functions inside a useEffect
3+
* @author Cyril Bonaccini
4+
*/
5+
"use strict";
6+
7+
//------------------------------------------------------------------------------
8+
// Requirements
9+
//------------------------------------------------------------------------------
10+
11+
const rule = require("../../../lib/rules/require-named-effect"),
12+
RuleTester = require("eslint").RuleTester;
13+
14+
//------------------------------------------------------------------------------
15+
// Tests
16+
//------------------------------------------------------------------------------
17+
18+
const ruleTester = new RuleTester({
19+
parser: require.resolve("@typescript-eslint/parser"),
20+
});
21+
22+
const valid = [
23+
`useEffect(function namedFunction() {}, []);`,
24+
`useEffect(theNameOfAFunction(), []);`,
25+
`useEffect(() => theNameOfAFunction(), []);`,
26+
`useEffect(() => {
27+
theOnlyChildIsAFunctionCall();
28+
}, []);`,
29+
];
30+
31+
const invalid = [
32+
`useEffect(() => {}, []);`,
33+
`useEffect(() => {
34+
const t = 1;
35+
disallowTwoThings(t);
36+
}, []);`,
37+
];
38+
39+
ruleTester.run("require-named-effect", rule, {
40+
valid,
41+
invalid: invalid.map((code) => ({
42+
code,
43+
errors: ["Complex effects must be a named function."],
44+
})),
45+
});

0 commit comments

Comments
 (0)