-
Notifications
You must be signed in to change notification settings - Fork 16.3k
chore: remove deprecated react-hot-loader #36433
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
base: master
Are you sure you want to change the base?
Conversation
|
CodeAnt AI is reviewing your PR. |
scripts/check-dependencies.py
Outdated
| recommendation = 'investigate' | ||
| elif dependents: | ||
| status = 'transitive-only' | ||
| recommendation = 'remove' # Can be removed if available through dependents |
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.
Suggestion: Logic error: when other packages depend on this package the code marks the package as removable. If dependents is non-empty the package is required by other packages and should not be recommended for removal; change the recommendation to keep (or investigate) instead of remove. [logic error]
Severity Level: Minor
| recommendation = 'remove' # Can be removed if available through dependents | |
| recommendation = 'keep' # Required by other packages; do not recommend removal automatically |
Why it matters? ⭐
The current logic flips the intent: if other packages depend on this package (dependents is non-empty),
the package is required transitively and should not be auto-recommended for removal. Changing the
recommendation to 'keep' (or at least 'investigate') fixes a real logic bug that would otherwise
produce unsafe/incorrect removal suggestions.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** scripts/check-dependencies.py
**Line:** 174:174
**Comment:**
*Logic Error: Logic error: when other packages depend on this package the code marks the package as removable. If `dependents` is non-empty the package is required by other packages and should not be recommended for removal; change the recommendation to keep (or investigate) instead of remove.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
scripts/check-dependencies.py
Outdated
| }, | ||
| 'transitive_usage': { | ||
| 'required_by': dependents[:5], # Limit to first 5 dependents | ||
| 'can_be_removed': len(dependents) > 0 |
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.
Suggestion: Incorrect boolean for transitive removal: can_be_removed is set true when there are dependents, which is backwards — if other packages require this package it should not be marked removable. Use a flag that reflects whether there are no dependents. [logic error]
Severity Level: Minor
| 'can_be_removed': len(dependents) > 0 | |
| 'can_be_removed': len(dependents) == 0 |
Why it matters? ⭐
The boolean is inverted: can_be_removed should be true only when there are NO dependents. As written,
packages that are required by other packages are incorrectly flagged removable. Fixing this corrects
the transitive usage reporting and aligns the field with the actual semantics.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** scripts/check-dependencies.py
**Line:** 190:190
**Comment:**
*Logic Error: Incorrect boolean for transitive removal: `can_be_removed` is set true when there are dependents, which is backwards — if other packages require this package it should not be marked removable. Use a flag that reflects whether there are no dependents.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Nitpicks 🔍
|
|
CodeAnt AI finished reviewing your PR. |
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.
Code Review Agent Run #b0d8a2
Actionable Suggestions - 2
-
scripts/check-dependencies.py - 2
- Incomplete Import Detection · Line 43-49
- Subprocess security and check argument issues · Line 71-74
Additional Suggestions - 2
-
scripts/check-dependencies.py - 2
-
Missing License Header · Line 1-16New files in the Superset codebase must include the Apache Software Foundation license header. This script is missing the required header, which should be added at the top before the shebang line.
-
Missing Type Hint · Line 216-216The main function should have a return type annotation for consistency with type hint requirements in new Python code.
Code suggestion
@@ -216,1 +216,1 @@ - def main(): + def main() -> None:
-
Review Details
-
Files reviewed - 5 · Commit Range:
d7c0d9b..d7c0d9b- scripts/check-dependencies.py
- superset-frontend/babel.config.js
- superset-frontend/src/preamble.ts
- superset-frontend/src/views/App.tsx
- superset-frontend/webpack.config.js
-
Files skipped - 3
- AGENTS/dependency-checker.md - Reason: Filter setting
- superset-frontend/package-lock.json - Reason: Filter setting
- superset-frontend/package.json - Reason: Filter setting
-
Tools
- Eslint (Linter) - ✔︎ Successful
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Default Agent You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.
Documentation & Help
scripts/check-dependencies.py
Outdated
| patterns = [ | ||
| f"from ['\"]{{}}['\"]", | ||
| f"require\\(['\"]{{}}['\"]", | ||
| f"import.*['\"]{{}}['\"]", | ||
| f"from ['\"]{{}}/.+['\"]", # Subpath imports | ||
| f"require\\(['\"]{{}}/.+['\"]", # Subpath requires | ||
| ] |
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.
The script misses dynamic imports like import('package'), which are common in the frontend for lazy loading. This could lead to false positives for unused dependencies.
Code suggestion
Check the AI-generated fix before applying
| patterns = [ | |
| f"from ['\"]{{}}['\"]", | |
| f"require\\(['\"]{{}}['\"]", | |
| f"import.*['\"]{{}}['\"]", | |
| f"from ['\"]{{}}/.+['\"]", # Subpath imports | |
| f"require\\(['\"]{{}}/.+['\"]", # Subpath requires | |
| ] | |
| patterns = [ | |
| f"from ['\"]{{}}['\"]", | |
| f"require\\(['\"]{{}}['\"]", | |
| f"import.*['\"]{{}}['\"]", | |
| f"from ['\"]{{}}/.+['\"]", # Subpath imports | |
| f"require\\(['\"]{{}}/.+['\"]", # Subpath requires | |
| f"import\\(['\"]{{}}['\"]", # Dynamic imports | |
| f"import\\(['\"]{{}}/.+['\"]", # Dynamic subpath imports | |
| ] |
Code Review Run #b0d8a2
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
scripts/check-dependencies.py
Outdated
| result = subprocess.run( | ||
| ["grep", "-r", "-l", "-E", search_pattern, str(src_path)], | ||
| capture_output=True, | ||
| text=True |
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.
Subprocess call to grep lacks explicit check argument (PLW1510) and has security concerns (S603, S607). Add check=False and use full path for executables.
Code suggestion
Check the AI-generated fix before applying
| result = subprocess.run( | |
| ["grep", "-r", "-l", "-E", search_pattern, str(src_path)], | |
| capture_output=True, | |
| text=True | |
| result = subprocess.run( | |
| ["grep", "-r", "-l", "-E", search_pattern, str(src_path)], | |
| capture_output=True, | |
| text=True, | |
| check=False, |
Code Review Run #b0d8a2
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
React Refresh is already configured and working via SWC loader. Removing the deprecated react-hot-loader package and all references. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
d7c0d9b to
12abbdc
Compare
User description
Summary
This PR removes the deprecated
react-hot-loaderpackage since React Refresh is already configured and working in the project via the SWC loader.Changes:
react-hot-loaderdependency from package.jsonBefore/After
Before: Project had both react-hot-loader (deprecated) and React Refresh configured
After: Only React Refresh is used for hot module replacement
Testing Instructions
npm run dev-server✅ Tested locally: Dev server compiles successfully and HMR works as expected.
Additional Information
React Refresh has been the recommended HMR solution for React since React 17. The react-hot-loader package is deprecated and no longer maintained. This migration simplifies our development setup by removing unnecessary dependencies.
🤖 Generated with Claude Code
CodeAnt-AI Description
Remove deprecated react-hot-loader and add dependency-checker tool
What Changed
Impact
✅ No deprecated HMR warnings in dev server✅ Smaller dev vendor bundle during development✅ Easier detection of unused frontend dependencies💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.