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

Rule no watch #51

Merged
merged 12 commits into from
Oct 6, 2021
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,4 @@ To configure individual rules:
- [effector/no-useless-methods](/rules/no-useless-methods/no-useless-methods.md)
- [effector/no-ambiguity-target](/rules/no-ambiguity-target/no-ambiguity-target.md)
- [effector/prefer-sample-over-forward-with-mapping](/rules/prefer-sample-over-forward-with-mapping/prefer-sample-over-forward-with-mapping.md)
- [effector/@typescript-no-watch](/rules/@typescript-no-watch/@typescript-no-watch.md)
igorkamyshev marked this conversation as resolved.
Show resolved Hide resolved
1 change: 1 addition & 0 deletions config/recommended.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,6 @@ module.exports = {
"effector/no-unnecessary-duplication": "warn",
"effector/prefer-sample-over-forward-with-mapping": "warn",
"effector/no-ambiguity-target": "warn",
"effector/@typescript-no-watch": "warn",
},
};
3 changes: 2 additions & 1 deletion index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@ module.exports = {
"prefer-sample-over-forward-with-mapping": require("./rules/prefer-sample-over-forward-with-mapping/prefer-sample-over-forward-with-mapping"),
"no-useless-methods": require("./rules/no-useless-methods/no-useless-methods"),
"no-ambiguity-target": require("./rules/no-ambiguity-target/no-ambiguity-target"),
"@typescript-no-watch": require("./rules/@typescript-no-watch/@typescript-no-watch"),
},
configs: {
recommended: require("./config/recommended"),
}
},
};
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,4 @@
"dependencies": {
"prettier": "^2.3.2"
}
}
}
64 changes: 64 additions & 0 deletions rules/@typescript-no-watch/@typescript-no-watch.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
const {
traverseNestedObjectNode,
} = require("../../utils/traverse-nested-object-node");

module.exports = {
meta: {
type: "suggestion",
docs: {
description: "Avoid `.watch` calls on any Effector unit or operator",
category: "Quality",
recommended: true,
},
messages: {
abusiveCall:
"Method `.watch` leads to imperative code. Try to replace it with operators (`sample`, `guard`, etc) or use the `target` parameter of the operators.",
},
schema: [],
},
create(context) {
const { parserServices } = context;

return {
CallExpression(node) {
const methodName = node.callee?.property?.name;

if (methodName !== "watch") {
return;
}

const object = traverseNestedObjectNode(node.callee?.object);
const objectName = object?.name;
const calleeName = object?.callee?.name;

if (!objectName && !["sample", "guard"].includes(calleeName)) {
igorkamyshev marked this conversation as resolved.
Show resolved Hide resolved
return;
}

// TypeScript-way
if (parserServices.hasFullTypeInformation) {
const checker = parserServices.program.getTypeChecker();
const originalNode = parserServices.esTreeNodeToTSNodeMap.get(object);
const type = checker.getTypeAtLocation(originalNode);

const isEffectorUnit =
["Effect", "Event", "Store"].includes(type?.symbol?.escapedName) &&
type?.symbol?.parent?.escapedName?.includes("effector");

if (!isEffectorUnit) {
return;
}

reportGetStateCall({ context, node });
}
},
};
},
};

function reportGetStateCall({ context, node }) {
context.report({
node,
messageId: "abusiveCall",
});
}
108 changes: 108 additions & 0 deletions rules/@typescript-no-watch/@typescript-no-watch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
# effector/@typescript-no-watch

igorkamyshev marked this conversation as resolved.
Show resolved Hide resolved
Method `.watch` leads to imperative code. Try replacing it with operators (`forward`, `sample`, etc) or use the `target` parameter of the operators.

```ts
igorkamyshev marked this conversation as resolved.
Show resolved Hide resolved
const myFx = createEffect();
const myEvent = createEvent();

// 👍 good solution
forward({
from: myFx,
to: myEvent,
});
forward({
from: myFx.done,
to: myEvent,
});
forward({
from: myFx.fail,
to: myEvent,
});
forward({
from: myFx.doneData,
to: myEvent,
});
forward({
from: myFx.failData,
to: myEvent,
});
forward({
from: myFx.finally,
to: myEvent,
});

// 👎 bad solution
myFx.watch(myEvent);
myFx.done.watch(myEvent);
myFx.fail.watch(myEvent);
myFx.doneData.watch(myEvent);
myFx.failData.watch(myEvent);
myFx.finally.watch(myEvent);
```

```ts
const myFx = createEffect();
const myEvent = createEvent();

// 👍 good solution
forward({
from: myEvent,
to: myFx,
});

// 👎 bad solution
myEvent.watch(async () => {
await myFx();
});
```

```ts
const $awesome = createStore();
const myEvent = createEvent();

// 👍 good solution
forward({
from: $awesome.updates,
to: myEvent,
});

// 👎 bad solution
$awesome.updates.watch(myEvent);
```

```ts
const myFx = createEffect();
const myEvent = createEvent();

// 👍 good solution
guard({
clock: myEvent,
filter: Boolean,
target: myFx,
});

// 👎 bad solution
guard({
clock: myEvent,
filter: Boolean,
}).watch(myFx);
```

```ts
const myFx = createEffect();
const myEvent = createEvent();

// 👍 good solution
sample({
clock: myEvent,
fn: Identity,
target: myFx,
});

// 👎 bad solution
sample({
clock: myEvent,
fn: Identity,
}).watch(myFx);
```
7 changes: 7 additions & 0 deletions rules/@typescript-no-watch/examples/correct.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
const event = {
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 add some smoke cases to correct test. E.g. sample({ ... }), `forward({...}), etc.

watch() {
return {};
},
};

export const value = event.watch();
11 changes: 11 additions & 0 deletions rules/@typescript-no-watch/examples/incorrect-with-effect.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { createEffect } from "effector";

const watcher = <T>(x: T) => x;
const watchableFx = createEffect();

watchableFx.watch(watcher);
watchableFx.done.watch(watcher);
watchableFx.fail.watch(watcher);
watchableFx.doneData.watch(watcher);
watchableFx.failData.watch(watcher);
watchableFx.finally.watch(watcher);
6 changes: 6 additions & 0 deletions rules/@typescript-no-watch/examples/incorrect-with-event.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { createEvent } from "effector";

const watcher = <T>(x: T) => x;
const watchableEvent = createEvent();

watchableEvent.watch(watcher);
9 changes: 9 additions & 0 deletions rules/@typescript-no-watch/examples/incorrect-with-guard.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { createEvent, guard } from "effector";

const watcher = <T>(x: T) => x;
const myEvent = createEvent();

guard({
clock: myEvent,
filter: () => Math.random() > 0.5,
}).watch(watcher);
9 changes: 9 additions & 0 deletions rules/@typescript-no-watch/examples/incorrect-with-sample.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { createEvent, sample } from "effector";

const watcher = <T>(x: T) => x;
const myEvent = createEvent();

sample({
clock: myEvent,
fn: () => true,
}).watch(watcher);
7 changes: 7 additions & 0 deletions rules/@typescript-no-watch/examples/incorrect-with-store.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { createStore } from "effector";

const watcher = <T>(x: T) => x;
const $watchable = createStore(0);

$watchable.watch(watcher);
$watchable.updates.watch(watcher);
101 changes: 101 additions & 0 deletions rules/@typescript-no-watch/no-watch.ts.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
const { RuleTester } =
require("@typescript-eslint/experimental-utils").ESLintUtils;
const { join } = require("path");

const { readExample } = require("../../utils/read-example");
const rule = require("./@typescript-no-watch");

const ruleTester = new RuleTester({
parser: "@typescript-eslint/parser",
parserOptions: {
ecmaVersion: 2020,
sourceType: "module",
project: "./tsconfig.json",
tsconfigRootDir: join(__dirname, ".."),
},
});

const readExampleForTheRule = (name) => ({
code: readExample(__dirname, name),
filename: join(__dirname, "examples", name),
});

ruleTester.run("effector/no-watch.ts.test", rule, {
valid: ["correct.ts"].map(readExampleForTheRule),

invalid: [
...["incorrect-with-effect.ts"]
.map(readExampleForTheRule)
.map((result) => ({
...result,
errors: [
{
messageId: "abusiveCall",
type: "CallExpression",
},
{
messageId: "abusiveCall",
type: "CallExpression",
},
{
messageId: "abusiveCall",
type: "CallExpression",
},
{
messageId: "abusiveCall",
type: "CallExpression",
},
{
messageId: "abusiveCall",
type: "CallExpression",
},
{
messageId: "abusiveCall",
type: "CallExpression",
},
],
})),
...["incorrect-with-event.ts"].map(readExampleForTheRule).map((result) => ({
...result,
errors: [
{
messageId: "abusiveCall",
type: "CallExpression",
},
],
})),
...["incorrect-with-guard.ts"].map(readExampleForTheRule).map((result) => ({
...result,
errors: [
{
messageId: "abusiveCall",
type: "CallExpression",
},
],
})),
...["incorrect-with-sample.ts"]
.map(readExampleForTheRule)
.map((result) => ({
...result,
errors: [
{
messageId: "abusiveCall",
type: "CallExpression",
},
],
})),
...["incorrect-with-store.ts"].map(readExampleForTheRule).map((result) => ({
...result,
errors: [
{
messageId: "abusiveCall",
type: "CallExpression",
},
{
messageId: "abusiveCall",
type: "CallExpression",
},
],
})),
],
});