Skip to content

Commit

Permalink
Fix #3908: mobx/missing-observer rule false positive with forwardRef (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
urugator authored Jul 24, 2024
1 parent 937d470 commit 638533e
Show file tree
Hide file tree
Showing 4 changed files with 126 additions and 81 deletions.
5 changes: 5 additions & 0 deletions .changeset/fifty-tools-reply.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"eslint-plugin-mobx": patch
---

mobx/missing-observer rule false positive with forwardRef #3908
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -74,5 +74,6 @@
"hooks": {
"pre-commit": "pretty-quick --staged"
}
}
},
"packageManager": "yarn@1.22.21+sha1.1959a18351b811cdeedbd484a8f86c3cc3bbaf72"
}
58 changes: 33 additions & 25 deletions packages/eslint-plugin-mobx/preview/missing-observer.js
Original file line number Diff line number Diff line change
@@ -1,32 +1,40 @@
/* eslint mobx/missing-observer: "error" */

function Named() { }
const foo = function Named() { }
const Anonym = function () { };
const Arrow = () => { };
function Named() {}
const named = function Named() {}
const namedRef = forwardRef(function Named() {})
const Anonym = function () {}
const AnonymRef = forwardRef(function () {})
const Arrow = () => {}
const ArrowRef = forwardRef(() => {})

observer(function Named() { });
const foo = observer(function Named() { })
const Anonym = observer(function () { });
const Arrow = observer(() => { });
observer(function Named() {})
observer(forwardRef(function Named() {}))
const namedObs = observer(function Named() {})
const namedRefObs = observer(forwardRef(function Named() {}))
const AnonymObs = observer(function () {})
const AnonymRefObs = observer(forwardRef(function () {}))
const ArrowObs = observer(() => {})
const ArrowRefObs = observer(forwardRef(() => {}))

function notCmp() { }
const notCmp = function notCmp() { }
const notCmp = function () { }
const notCmp = () => { }
function notCmp() {}
const notCmp = function notCmp() {}
const notCmp = function () {}
const notCmp = () => {}
const notCmp = forwardRef(() => {})

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 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 { }
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 { })
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 {})
141 changes: 86 additions & 55 deletions packages/eslint-plugin-mobx/src/missing-observer.js
Original file line number Diff line number Diff line change
@@ -1,63 +1,94 @@
'use strict';
"use strict"

function create(context) {
const sourceCode = context.getSourceCode();
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;
}
return {
"FunctionDeclaration,FunctionExpression,ArrowFunctionExpression,ClassDeclaration,ClassExpression":
cmp => {
if (
cmp.parent &&
cmp.parent.type === "CallExpression" &&
cmp.parent.callee.name === "observer"
) {
// observer(...)
return
}
let forwardRef =
cmp.parent &&
cmp.parent.type === "CallExpression" &&
cmp.parent.callee.name === "forwardRef"
? cmp.parent
: undefined
if (
forwardRef &&
forwardRef.parent &&
forwardRef.parent.type === "CallExpression" &&
forwardRef.parent.callee.name === "observer"
) {
// forwardRef(observer(...))
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,
})
},
};
const cmpOrForwardRef = forwardRef || cmp
let name = cmp.id?.name
// If anonymous try to infer name from variable declaration
if (!name && cmpOrForwardRef.parent?.type === "VariableDeclarator") {
name = cmpOrForwardRef.parent.id.name
}
if (cmp.type.startsWith("Class")) {
// Must extend Component or React.Component
const { superClass } = cmp
if (!superClass) {
// not a component
return
}
const superClassText = sourceCode.getText(superClass)
if (superClassText !== "Component" && superClassText !== "React.Component") {
// not a component
return
}
} else {
// Name must start with uppercase letter
if (!name?.charAt(0).match(/^[A-Z]$/)) {
// not a component
return
}
}

const fix = fixer => {
return [
fixer.insertTextBefore(
sourceCode.getFirstToken(cmpOrForwardRef),
(name && cmp.type.endsWith("Declaration") ? `const ${name} = ` : "") +
"observer("
),
fixer.insertTextAfter(sourceCode.getLastToken(cmpOrForwardRef), ")")
]
}
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`.",
meta: {
type: "problem",
fixable: "code",
docs: {
description: "prevents missing `observer` on react component",
recommended: true
},
messages: {
missingObserver: "Component `{{ name }}` is missing `observer`."
}
},
},
create,
};
create
}

0 comments on commit 638533e

Please sign in to comment.