Skip to content

Commit

Permalink
chore: adding eslint rules to warn new object and function props (#35757
Browse files Browse the repository at this point in the history
)

## Description
Adding eslint rules to prevent new object props so that the performance
of components declared as pure components does not regress.

Fixes #`Issue Number`  
_or_  
Fixes `Issue URL`
> [!WARNING]  
> _If no issue exists, please create an issue first, and check with the
maintainers if the issue is valid._

## Automation

/ok-to-test tags="@tag.Sanity"

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/10528770094>
> Commit: c86fb95
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=10528770094&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.Sanity`
> Spec:
> <hr>Fri, 23 Aug 2024 16:33:49 UTC
<!-- end of auto-generated comment: Cypress test results  -->


## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [ ] No


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit


- **New Features**
- Integrated a performance-focused linting plugin for React to improve
the optimization of components.

- **Enhancements**
- Implemented new linting rules to prevent performance issues related to
the usage of new object instances as props in JSX.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
vsvamsi1 authored Aug 24, 2024
1 parent 4e68f16 commit 9889117
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 1 deletion.
15 changes: 14 additions & 1 deletion app/client/.eslintrc.base.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
"cypress",
"testing-library",
"jest",
"@appsmith"
"@appsmith",
"react-perf"
],
"extends": [
"plugin:react/recommended", // Uses the recommended rules from @eslint-plugin-react
Expand All @@ -26,6 +27,7 @@
"plugin:testing-library/react",
"plugin:react-hooks/recommended",
"plugin:@appsmith/recommended",
"plugin:react-perf/recommended",
// Note: Please keep this as the last config to make sure that this (and by extension our .prettierrc file) overrides all configuration above it
// https://www.npmjs.com/package/eslint-plugin-prettier#recommended-configuration
"plugin:prettier/recommended"
Expand Down Expand Up @@ -67,6 +69,17 @@
],
"no-console": "error",
"no-debugger": "error",
"react-perf/jsx-no-new-array-as-prop": "warn",
"react-perf/jsx-no-new-function-as-prop": "warn",
"react-perf/jsx-no-jsx-as-prop": "warn",
"react-perf/jsx-no-new-object-as-prop": [
"warn",
{
// we are disabling this rule here for native component since it won't make much difference in performance
// we want to target react components more since there is possibility they can be pure components and new istance props should be avoided for them
"nativeAllowList": "all"
}
],
"@typescript-eslint/no-restricted-imports": [
"error",
{
Expand Down
1 change: 1 addition & 0 deletions app/client/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,7 @@
"eslint-plugin-prettier": "^5.0.0",
"eslint-plugin-react": "^7.33.2",
"eslint-plugin-react-hooks": "^4.6.0",
"eslint-plugin-react-perf": "^3.3.2",
"eslint-plugin-sort-destructure-keys": "^1.5.0",
"eslint-plugin-storybook": "^0.6.15",
"eslint-plugin-testing-library": "^6.2.0",
Expand Down
10 changes: 10 additions & 0 deletions app/client/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -13322,6 +13322,7 @@ __metadata:
eslint-plugin-prettier: ^5.0.0
eslint-plugin-react: ^7.33.2
eslint-plugin-react-hooks: ^4.6.0
eslint-plugin-react-perf: ^3.3.2
eslint-plugin-sort-destructure-keys: ^1.5.0
eslint-plugin-storybook: ^0.6.15
eslint-plugin-testing-library: ^6.2.0
Expand Down Expand Up @@ -18900,6 +18901,15 @@ __metadata:
languageName: node
linkType: hard

"eslint-plugin-react-perf@npm:^3.3.2":
version: 3.3.2
resolution: "eslint-plugin-react-perf@npm:3.3.2"
peerDependencies:
eslint: ^3.0.0 || ^4.0.0 || ^5.0.0 || ^6.0.0 || ^7.0.0 || ^8.0.0
checksum: 236982166bfb598c758a2ef0ba69e5173521c0f6481a8767aca5d29cf32928789d4bc1c7316aa274706baab820828bdb36336508c492071916bdb234a4c99417
languageName: node
linkType: hard

"eslint-plugin-react@npm:^7.27.1, eslint-plugin-react@npm:^7.33.2":
version: 7.33.2
resolution: "eslint-plugin-react@npm:7.33.2"
Expand Down

0 comments on commit 9889117

Please sign in to comment.