eslint: missing-observer, no-anonymous-observer (#3219)

.changeset/curly-vans-enjoy.md

@@ -0,0 +1,6 @@
+---
+"eslint-plugin-mobx": patch
+---
+
+Add [`mobx/missing-observer`](https://github.com/mobxjs/mobx/tree/main/packages/eslint-plugin-mobx#mobxmissing-observer),
+[`mobx/no-anonymous-observer`](https://github.com/mobxjs/mobx/tree/main/packages/eslint-plugin-mobx#mobxno-anonymous-observer) rules,

docs/configuration.md

@@ -68,10 +68,15 @@ configure({
 At some point you will discover that this level of strictness can be pretty annoying.
 It is fine to disable these rules to gain productivity once you are sure you (and your colleagues) grokked the mental model of MobX.
 
-Also, occassionally you will have a case where you have to supress the warnings triggered by these rules (for example by wrapping in `runInAction`).
+Also, occasionally you will have a case where you have to supress the warnings triggered by these rules (for example by wrapping in `runInAction`).
 That is fine, there are good exceptions to these recommendations.
 Don't be fundamentalistic about them.
 
+Make sure to also try our [`eslint` plugin](https://github.com/mobxjs/mobx/blob/main/packages/eslint-plugin-mobx/README.md).
+While some problems are discoverable statically, others are detectable only at runtime.
+The plugin is intended to complement these rules, not to replace them.
+The autofix feature can also help with the boilerplate code.
+
 #### `enforceActions`
 
 The goal of _enforceActions_ is that you don't forget to wrap event handlers in [`action`](actions.md).

packages/eslint-plugin-mobx/README.md

@@ -21,9 +21,11 @@ module.exports = {
     // ...or specify and customize individual rules:
     rules: {
       // these values are the same as recommended
-      'mobx/exhaustive-make-observable': 'warn',      
+      'mobx/exhaustive-make-observable': 'warn',
+      'mobx/unconditional-make-observable': 'error',
       'mobx/missing-make-observable': 'error',
-      'mobx/unconditional-make-observable': 'error',      
+      'mobx/no-missing-observer': 'warn',
+      'mobx/no-anonymous-observer': 'warn',     
     },
 };
 ```
@@ -33,16 +35,54 @@ module.exports = {
 ### mobx/exhaustive-make-observable
 
 Makes sure that `makeObservable` annotates all fields defined on class or object literal.<br>
-Autofix adds `field: true` for each missing field.<br>
+**Autofix** adds `field: true` for each missing field.<br>
 To exclude a field, annotate it using `field: false`.<br>
 Does not support fields introduced by constructor (`this.foo = 5`).<br>
 Does not warn about annotated non-existing fields (there is a runtime check, but the autofix removing the field could be handy...).
 
 ### mobx/missing-make-observable
 
 *When using decorators (eg `@observable foo = 5`)*, makes sure that `makeObservable(this)` is called in a constructor.<br>
-Autofix creates a constructor if necessary and adds `makeObservable(this)` at it's end.
+**Autofix** creates a constructor if necessary and adds `makeObservable(this)` at it's end.
 
 ### mobx/unconditional-make-observable
 
-Makes sure the `make(Auto)Observable(this)` is called unconditionally inside a constructor.
+Makes sure the `make(Auto)Observable(this)` is called unconditionally inside a constructor.
+
+### mobx/missing-observer
+
+Makes sure every React component is wrapped with `observer`. A React component is considered to be any *class* extending from `Component` or `React.Component` and any *function* which name has the first letter capitalized (for anonymous functions the name is inferred from variable). These are all considered components:
+```javascript
+class Cmp extends React.Component { }
+class Cmp extends Component { }
+const Cmp = class extends React.Component { }
+const Cmp = class extends Component { }
+class extends Component { }
+class extends React.Component { }
+
+function Named() { }
+const foo = function Named() { }
+const Anonym = function () { };
+const Arrow = () => { };
+```
+**Autofix** wraps the component with `observer` and if necessary declares a constant of the same name: `const Name = observer(function Name() {})`.
+It's a bit opinionated and can lead to a lot of false positives depending on your conventions. You will probably want to combine this rule with `overrides` option, eg:
+```javascript
+// .eslintrc.js
+"overrides": [
+  {
+    "files": ["*.jsx"],
+    "rules": {
+      "mobx/missing-observer": "error"
+    }
+  }
+]
+```
+
+### mobx/no-anonymous-observer
+
+Forbids anonymous functions or classes as `observer` components. 
+Improves debugging experience and [avoids problem with inability to customize `displayName`](https://github.com/mobxjs/mobx/issues/2721).
+Plays nice with `eslint-plugin-react-hooks` and `mobx/missing-observer` as both of these don't not recognize anonymous function as component.
+**Autofix** infers the name from variable if possible.
+

packages/eslint-plugin-mobx/__tests__/TODO-missing-observer.js

@@ -0,0 +1,15 @@
+import { RuleTester } from "eslint";
+
+import rule from "../src/missing-observer.js";
+
+const tester = new RuleTester({
+  parser: require.resolve('@typescript-eslint/parser'),
+  parserOptions: {}
+});
+
+tester.run("missing-observer", rule, {
+  valid: [
+    { code: `` },
+  ],
+  invalid: [],
+});

packages/eslint-plugin-mobx/__tests__/TODO-no-anonymous-observer.js

@@ -0,0 +1,15 @@
+import { RuleTester } from "eslint";
+
+import rule from "../src/no-anonymous-observer.js";
+
+const tester = new RuleTester({
+  parser: require.resolve('@typescript-eslint/parser'),
+  parserOptions: {}
+});
+
+tester.run("no-anonymous-observer", rule, {
+  valid: [
+    { code: `` },
+  ],
+  invalid: [],
+});

packages/eslint-plugin-mobx/package.json

@@ -36,7 +36,7 @@
         "rollup": "^2.60.2",
         "@rollup/plugin-babel": "^5.3.0",
         "@rollup/plugin-commonjs": "^21.0.1",
-        "@rollup/plugin-node-resolve": "13.0.6"
+        "@rollup/plugin-node-resolve": "13.0.6"        
     },
     "keywords": [
         "eslint",

packages/eslint-plugin-mobx/preview/.eslintrc.js

@@ -19,5 +19,6 @@ module.exports = {
     "rules": {
         //'mobx/exhaustive-make-observable': 'off',
         //'mobx/unconditional-make-observable': 'off',
+        //'mobx/missing-observer': 'off',
     }
 };

packages/eslint-plugin-mobx/preview/make-observable.js

@@ -1,3 +1,7 @@
+/* eslint mobx/exhaustive-make-observable: "error" */
+/* eslint mobx/missing-make-observable: "error" */
+/* eslint mobx/unconditional-make-observable: "error" */
+
 makeObservable();
 makeObservable(foo, {});
 makeObservable(this, {});

packages/eslint-plugin-mobx/preview/missing-observer.js

@@ -0,0 +1,32 @@
+/* eslint mobx/missing-observer: "error" */
+
+function Named() { }
+const foo = function Named() { }
+const Anonym = function () { };
+const Arrow = () => { };
+
+observer(function Named() { });
+const foo = observer(function Named() { })
+const Anonym = observer(function () { });
+const Arrow = observer(() => { });
+
+function notCmp() { }
+const notCmp = function notCmp() { }
+const notCmp = function () { }
+const notCmp = () => { }
+
+class Cmp extends React.Component { }
+class Cmp extends Component { }
+const Cmp = class extends React.Component { }
+const Cmp = class extends Component { }
+class extends Component { }
+class extends React.Component { }
+
+class NotCmp { }
+class NotCmp extends Foo { }
+class NotCmp extends React.Foo { }
+
+const Cmp = observer(class Cmp extends React.Component { })
+const Cmp = observer(class Cmp extends Component { })
+const Cmp = observer(class extends React.Component { })
+const Cmp = observer(class extends Component { })

packages/eslint-plugin-mobx/preview/no-anonymous-observer.js

@@ -0,0 +1,12 @@
+/* eslint mobx/no-anonymous-observer: "error" */
+
+observer(() => { });
+observer(function () { });
+observer(class { });
+
+const Cmp = observer(() => { });
+const Cmp = observer(function () { });
+const Cmp = observer(class { });
+
+observer(function Name() { });
+observer(class Name { });

packages/eslint-plugin-mobx/src/index.js

@@ -3,6 +3,8 @@
 const exhaustiveMakeObservable = require('./exhaustive-make-observable.js');
 const unconditionalMakeObservable = require('./unconditional-make-observable.js');
 const missingMakeObservable = require('./missing-make-observable.js');
+const missingObserver = require('./missing-observer');
+const noAnonymousObserver = require('./no-anonymous-observer.js');
 
 module.exports = {
   configs: {
@@ -12,12 +14,16 @@ module.exports = {
         'mobx/exhaustive-make-observable': 'warn',
         'mobx/unconditional-make-observable': 'error',
         'mobx/missing-make-observable': 'error',
+        'mobx/no-missing-observer': 'warn',
+        'mobx/no-anonymous-observer': 'warn',
       },
     },
   },
   rules: {
     'exhaustive-make-observable': exhaustiveMakeObservable,
     'unconditional-make-observable': unconditionalMakeObservable,
     'missing-make-observable': missingMakeObservable,
+    'missing-observer': missingObserver,
+    'no-anonymous-observer': noAnonymousObserver,
   }
 }

packages/eslint-plugin-mobx/src/missing-observer.js

@@ -0,0 +1,63 @@
+'use strict';
+
+function create(context) {
+  const sourceCode = context.getSourceCode();
+
+  return {
+    'FunctionDeclaration,FunctionExpression,ArrowFunctionExpression,ClassDeclaration,ClassExpression': cmp => {
+      // Already has observer
+      if (cmp.parent && cmp.parent.type === 'CallExpression' && cmp.parent.callee.name === 'observer') return;
+      let name = cmp.id?.name;
+      // If anonymous try to infer name from variable declaration      
+      if (!name && cmp.parent?.type === 'VariableDeclarator') {
+        name = cmp.parent.id.name;
+      }
+      if (cmp.type.startsWith('Class')) {
+        // Must extend Component or React.Component
+        const { superClass } = cmp;
+        if (!superClass) return;
+        const superClassText = sourceCode.getText(superClass);
+        if (superClassText !== 'Component' && superClassText !== 'React.Component') return;
+      } else {
+        // Name must start with uppercase letter
+        if (!name?.charAt(0).match(/^[A-Z]$/)) return;
+      }
+
+      const fix = fixer => {
+        return [
+          fixer.insertTextBefore(
+            sourceCode.getFirstToken(cmp),
+            (name && cmp.type.endsWith('Declaration') ? `const ${name} = ` : '') + 'observer(',
+          ),
+          fixer.insertTextAfter(
+            sourceCode.getLastToken(cmp),
+            ')',
+          ),
+        ]
+      }
+      context.report({
+        node: cmp,
+        messageId: 'missingObserver',
+        data: {
+          name: name || '<anonymous>',
+        },
+        fix,
+      })
+    },
+  };
+}
+
+module.exports = {
+  meta: {
+    type: 'problem',
+    fixable: 'code',
+    docs: {
+      description: 'prevents missing `observer` on react component',
+      recommended: true,
+    },
+    messages: {
+      missingObserver: "Component `{{ name }}` is missing `observer`.",
+    },
+  },
+  create,
+};

packages/eslint-plugin-mobx/src/no-anonymous-observer.js

@@ -0,0 +1,57 @@
+'use strict';
+
+function create(context) {
+  const sourceCode = context.getSourceCode();
+
+  return {
+    'CallExpression[callee.name="observer"]': observer => {
+      const cmp = observer.arguments[0];
+      if (!cmp) return;
+      if (cmp?.id?.name) return;
+
+      const fix = fixer => {
+        // Use name from variable for autofix 
+        const name = observer.parent?.type === 'VariableDeclarator'
+          ? observer.parent.id.name
+          : undefined;
+
+        if (!name) return;
+        if (cmp.type === 'ArrowFunctionExpression') {
+          const arrowToken = sourceCode.getTokenBefore(cmp.body);
+          return [
+            fixer.replaceText(arrowToken, ''),
+            fixer.insertTextBefore(cmp, `function ${name}`),
+          ]
+        }
+        if (cmp.type === 'FunctionExpression') {
+          const functionToken = sourceCode.getFirstToken(cmp);
+          return fixer.replaceText(functionToken, `function ${name}`);
+        }
+        if (cmp.type === 'ClassExpression') {
+          const classToken = sourceCode.getFirstToken(cmp);
+          return fixer.replaceText(classToken, `class ${name}`);
+        }
+      }
+      context.report({
+        node: cmp,
+        messageId: 'observerComponentMustHaveName',
+        fix,
+      })
+    },
+  };
+}
+
+module.exports = {
+  meta: {
+    type: 'problem',
+    fixable: 'code',
+    docs: {
+      description: 'forbids anonymous functions or classes as `observer` components',
+      recommended: true,
+    },
+    messages: {
+      observerComponentMustHaveName: "`observer` component must have a name.",
+    },
+  },
+  create,
+};