-
Notifications
You must be signed in to change notification settings - Fork 3.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add react-compiler eslint plugin #37139
Conversation
WalkthroughThe pull request introduces updates to the ESLint configuration and the package dependencies for the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
app/client/.eslintrc.base.json (2)
45-45
: Consider documenting the rule's purpose.
Adding a comment above the rule would help other developers understand why this warning-level rule was added and its relationship to the React Compiler integration.
+ // Enable React Compiler warnings to prepare for full integration
"react-compiler/react-compiler": "warn",
14-14
: Review beta dependency status.
The React Compiler ESLint plugin is currently in beta. Consider:
- Adding a TODO comment to update once a stable version is released
- Documenting any known limitations or issues
- Setting up a process to monitor for stable release
Also applies to: 45-45
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
⛔ Files ignored due to path filters (1)
app/client/yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (2)
- app/client/.eslintrc.base.json (2 hunks)
- app/client/package.json (1 hunks)
🔇 Additional comments (1)
app/client/package.json (1)
324-324
:
Consider pinning the beta version for stability.
Using an unpinned beta version ("eslint-plugin-react-compiler": "beta"
) could lead to unexpected breaking changes. Consider pinning to a specific beta version number.
Let's verify the package's latest beta version and maintenance status:
- "eslint-plugin-react-compiler": "beta",
+ "eslint-plugin-react-compiler": "0.1.0-beta.1",
✅ Verification successful
Update the version to the latest beta for stability
Based on the npm registry data, let's pin to the current beta version 19.0.0-beta-6fc168f-20241025
. This package is officially maintained by the React team and was recently published.
- "eslint-plugin-react-compiler": "beta",
+ "eslint-plugin-react-compiler": "19.0.0-beta-6fc168f-20241025"
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Get package information from npm
echo "Package Information:"
npm view eslint-plugin-react-compiler
# Check package download statistics for popularity
echo -e "\nDownload Statistics:"
npm view eslint-plugin-react-compiler downloads
Length of output: 1421
@@ -11,6 +11,7 @@ | |||
], | |||
"plugins": [ | |||
"react", | |||
"eslint-plugin-react-compiler", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Beta dependency poses production risk
The eslint-plugin-react-compiler is currently using a beta version (19.0.0-beta-6fc168f-20241025). Using beta dependencies in production code can lead to instability and unexpected behavior.
- Consider waiting for a stable release before integrating this plugin
- If beta version is required, document the decision and potential risks in the project documentation
- Set up a process to track and upgrade the plugin once a stable version is available
🔗 Analysis chain
Verify plugin installation and version.
Let's ensure the plugin is properly installed and its version is appropriate for production use.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the plugin is installed and get its version
jq '.devDependencies["eslint-plugin-react-compiler"]' app/client/package.json
Length of output: 86
Script:
#!/bin/bash
# Check if the plugin is published on npm and get its latest version
npm view eslint-plugin-react-compiler version 2>/dev/null || echo "Package not found"
# Search for any documentation or usage of this plugin in the codebase
rg -l "react-compiler" --type md
Length of output: 109
5cfdd21
to
89b1e8f
Compare
## Description Added react-compiler eslint plugin as the first step in preparation for adding React Compiler https://react.dev/learn/react-compiler#installing-eslint-plugin-react-compiler 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/11613472733> > Commit: 89b1e8f > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=11613472733&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Sanity` > Spec: > <hr>Thu, 31 Oct 2024 14:48:54 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 ## Summary by CodeRabbit - **New Features** - Enhanced linting capabilities with the addition of the `eslint-plugin-react-compiler` for improved React code quality. - **Chores** - Updated ESLint configuration to include a new rule for the React compiler, issuing warnings to maintain coding standards. - Added `eslint-plugin-react-compiler` as a new development dependency. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Description
Added react-compiler eslint plugin as the first step in preparation for adding React Compiler
https://react.dev/learn/react-compiler#installing-eslint-plugin-react-compiler
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
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/11613472733
Commit: 89b1e8f
Cypress dashboard.
Tags:
@tag.Sanity
Spec:
Thu, 31 Oct 2024 14:48:54 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Summary by CodeRabbit
New Features
eslint-plugin-react-compiler
for improved React code quality.Chores
eslint-plugin-react-compiler
as a new development dependency.