From 8368a20fcfd53557519adbb339040df9fed9eabd Mon Sep 17 00:00:00 2001 From: Fanny Tavart Date: Tue, 3 Sep 2024 16:43:00 +0200 Subject: [PATCH] BREAKING CHANGE: refactor -> create custom rule for import of useIsFocused to prevent from override by import eslint rule --- .../break-use-is-focused-import-rule.tsx | 4 +- .../docs/rules/no-use-is-focused.md | 17 +++++ .../eslint-plugin/lib/configs/performance.ts | 14 +--- packages/eslint-plugin/lib/rules/index.ts | 2 + .../lib/rules/no-use-is-focused.ts | 74 +++++++++++++++++++ .../tests/lib/rules/no-use-is-focused.test.ts | 25 +++++++ 6 files changed, 121 insertions(+), 15 deletions(-) create mode 100644 packages/eslint-plugin/docs/rules/no-use-is-focused.md create mode 100644 packages/eslint-plugin/lib/rules/no-use-is-focused.ts create mode 100644 packages/eslint-plugin/tests/lib/rules/no-use-is-focused.test.ts diff --git a/example-app/eslint-breaking-examples/break-use-is-focused-import-rule.tsx b/example-app/eslint-breaking-examples/break-use-is-focused-import-rule.tsx index d31d103f..c5167e50 100644 --- a/example-app/eslint-breaking-examples/break-use-is-focused-import-rule.tsx +++ b/example-app/eslint-breaking-examples/break-use-is-focused-import-rule.tsx @@ -1,7 +1,7 @@ // Save without formatting: [⌘ + K] > [S] -// This should trigger one error breaking eslint-plugin-react-native: -// no-restricted-imports +// This should trigger one error breaking custom performance rule: +// @bam.tech/no-use-is-focused import { useIsFocused } from "@react-navigation/native"; import { Text } from "react-native"; diff --git a/packages/eslint-plugin/docs/rules/no-use-is-focused.md b/packages/eslint-plugin/docs/rules/no-use-is-focused.md new file mode 100644 index 00000000..6d91a4da --- /dev/null +++ b/packages/eslint-plugin/docs/rules/no-use-is-focused.md @@ -0,0 +1,17 @@ +# Disallow importing `useIsFocused` from `@react-navigation/native` to encourage using `useFocusEffect` instead (`@bam.tech/no-use-is-focused`) + +💼 This rule is enabled in the `performance` config. + +🔧 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix). + + + +Prevents from using "useIsFocused" to avoid performance issues. "useFocusEffect" should be used instead. + +## Rule details + +Examples of **incorrect** code for this rule: + +```jsx +import { useIsFocused } from "@react-navigation/native"; +``` diff --git a/packages/eslint-plugin/lib/configs/performance.ts b/packages/eslint-plugin/lib/configs/performance.ts index 0ee98484..2a59ff8c 100644 --- a/packages/eslint-plugin/lib/configs/performance.ts +++ b/packages/eslint-plugin/lib/configs/performance.ts @@ -2,24 +2,12 @@ import { defineConfig } from "eslint-define-config"; export const performanceConfig = defineConfig({ rules: { - "no-restricted-imports": [ - "error", - { - paths: [ - { - name: "@react-navigation/native", - importNames: ["useIsFocused"], - message: - "Please use useFocusEffect instead of useIsFocused to avoid excessive rerenders.", - }, - ], - }, - ], "@bam.tech/no-animated-without-native-driver": "error", "@bam.tech/avoid-intl-number-format": "error", "@bam.tech/avoid-react-native-svg": "warn", "@bam.tech/no-flatlist": "error", "@bam.tech/no-react-navigation-stack": "error", + "@bam.tech/no-use-is-focused": "error", }, overrides: [ { diff --git a/packages/eslint-plugin/lib/rules/index.ts b/packages/eslint-plugin/lib/rules/index.ts index c3565b1c..26fc8098 100644 --- a/packages/eslint-plugin/lib/rules/index.ts +++ b/packages/eslint-plugin/lib/rules/index.ts @@ -7,6 +7,7 @@ import { preferUserEventRule } from "./prefer-user-event"; import { requireNamedEffectRule } from "./require-named-effect"; import { noFlatListImportRule } from "./no-flatlist"; import { noReactNavigationStackImportRule } from "./no-react-navigation-stack"; +import { noUseIsFocusedImportRule } from "./no-use-is-focused"; export default { "await-user-event": awaitUserEventRule, @@ -18,4 +19,5 @@ export default { "avoid-react-native-svg": avoidReactNativeSvgImportRule, "no-flatlist": noFlatListImportRule, "no-react-navigation-stack": noReactNavigationStackImportRule, + "no-use-is-focused": noUseIsFocusedImportRule, }; diff --git a/packages/eslint-plugin/lib/rules/no-use-is-focused.ts b/packages/eslint-plugin/lib/rules/no-use-is-focused.ts new file mode 100644 index 00000000..6d424049 --- /dev/null +++ b/packages/eslint-plugin/lib/rules/no-use-is-focused.ts @@ -0,0 +1,74 @@ +import type { Rule } from "eslint"; +import type { ImportDeclaration, CallExpression, Property } from "estree"; + +// Custom Rule: No Import of useIsFocused from @react-navigation/native +export const noUseIsFocusedImportRule: Rule.RuleModule = { + meta: { + type: "problem", + docs: { + description: + "Disallow importing `useIsFocused` from `@react-navigation/native` to encourage using `useFocusEffect` instead.", + category: "Best Practices", + recommended: true, + url: "https://github.com/bamlab/react-native-project-config/tree/main/packages/eslint-plugin/docs/rules/no-use-is-focused.md", + }, + messages: { + noUseIsFocusedImport: + "Please use 'useFocusEffect' instead of 'useIsFocused' to avoid excessive rerenders: 'useIsFocused' will trigger rerender both when the page goes in and out of focus.", + }, + schema: [], + fixable: "code", + }, + + create(context) { + return { + ImportDeclaration(node: ImportDeclaration) { + if (node.source.value === "@react-navigation/native") { + node.specifiers.forEach((specifier) => { + if ( + specifier.type === "ImportSpecifier" && + specifier.imported.name === "useIsFocused" + ) { + context.report({ + node: specifier, + messageId: "noUseIsFocusedImport", + }); + } + }); + } + }, + CallExpression(node: CallExpression) { + if ( + node.callee.type === "Identifier" && + node.callee.name === "require" && + node.arguments.length > 0 && + node.arguments[0].type === "Literal" && + node.arguments[0].value === "@react-navigation/native" + ) { + const ancestors = context.getAncestors(); + const parent = ancestors[ancestors.length - 1]; // Get the direct parent of the node + + if ( + parent.type === "VariableDeclarator" && + parent.id.type === "ObjectPattern" + ) { + const properties = parent.id.properties as Property[]; + const useIsFocusedProperty = properties.find( + (prop) => + prop.type === "Property" && + prop.key.type === "Identifier" && + prop.key.name === "useIsFocused", + ); + + if (useIsFocusedProperty) { + context.report({ + node: useIsFocusedProperty, + messageId: "noUseIsFocusedImport", + }); + } + } + } + }, + }; + }, +}; diff --git a/packages/eslint-plugin/tests/lib/rules/no-use-is-focused.test.ts b/packages/eslint-plugin/tests/lib/rules/no-use-is-focused.test.ts new file mode 100644 index 00000000..9c865a92 --- /dev/null +++ b/packages/eslint-plugin/tests/lib/rules/no-use-is-focused.test.ts @@ -0,0 +1,25 @@ +// Save without formatting: [⌘ + K] > [S] + +// This should trigger an error breaking eslint-plugin-bam-custom-rules: +// bam-custom-rules/no-use-is-focused + +import { noUseIsFocusedImportRule } from "../../../lib/rules/no-use-is-focused"; +import { RuleTester } from "eslint"; + +const ruleTester = new RuleTester({ + parser: require.resolve("@typescript-eslint/parser"), +}); + +const valid = [`import { useFocusEffect } from "@react-navigation/native";`]; + +const invalid = [`import { useIsFocused } from "@react-navigation/native";`]; + +ruleTester.run("no-use-is-focused", noUseIsFocusedImportRule, { + valid, + invalid: invalid.map((code) => ({ + code, + errors: [ + `Please use 'useFocusEffect' instead of 'useIsFocused' to avoid excessive rerenders: 'useIsFocused' will trigger rerender both when the page goes in and out of focus.`, + ], + })), +});