From 638533e592f4fda7663fa351447380f9d0917d14 Mon Sep 17 00:00:00 2001 From: urugator <11457665+urugator@users.noreply.github.com> Date: Wed, 24 Jul 2024 21:36:35 +0200 Subject: [PATCH] Fix #3908: mobx/missing-observer rule false positive with forwardRef (#3909) --- .changeset/fifty-tools-reply.md | 5 + package.json | 3 +- .../preview/missing-observer.js | 58 +++---- .../src/missing-observer.js | 141 +++++++++++------- 4 files changed, 126 insertions(+), 81 deletions(-) create mode 100644 .changeset/fifty-tools-reply.md diff --git a/.changeset/fifty-tools-reply.md b/.changeset/fifty-tools-reply.md new file mode 100644 index 000000000..4d527674f --- /dev/null +++ b/.changeset/fifty-tools-reply.md @@ -0,0 +1,5 @@ +--- +"eslint-plugin-mobx": patch +--- + +mobx/missing-observer rule false positive with forwardRef #3908 diff --git a/package.json b/package.json index fb07f1f18..68eaf2acb 100644 --- a/package.json +++ b/package.json @@ -74,5 +74,6 @@ "hooks": { "pre-commit": "pretty-quick --staged" } - } + }, + "packageManager": "yarn@1.22.21+sha1.1959a18351b811cdeedbd484a8f86c3cc3bbaf72" } diff --git a/packages/eslint-plugin-mobx/preview/missing-observer.js b/packages/eslint-plugin-mobx/preview/missing-observer.js index e76e69285..96f0fbd52 100644 --- a/packages/eslint-plugin-mobx/preview/missing-observer.js +++ b/packages/eslint-plugin-mobx/preview/missing-observer.js @@ -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 { }) \ No newline at end of file +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 {}) diff --git a/packages/eslint-plugin-mobx/src/missing-observer.js b/packages/eslint-plugin-mobx/src/missing-observer.js index 8be20f5d6..67450a7cf 100644 --- a/packages/eslint-plugin-mobx/src/missing-observer.js +++ b/packages/eslint-plugin-mobx/src/missing-observer.js @@ -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 || '', - }, - 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 || "" + }, + 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, -}; \ No newline at end of file + create +}