Skip to content

Conversation

@mheob
Copy link
Owner

@mheob mheob commented Oct 21, 2025

Summary

Updates React ESLint rules configuration to align with eslint-plugin-react-x recommendations and removes unused post-merge hook.

Changes

  • ESLint Config: Updated React rules in packages/eslint-config/src/configs/react.ts
    • Fixed comment to reference correct plugin name (eslint-plugin-react-x)
    • Re-enabled JSX rules: jsx-no-duplicate-props, jsx-uses-react, jsx-uses-vars
    • Added jsx-no-comment-textnodes rule
    • Consolidated no-comment-textnodes into jsx-no-comment-textnodes
    • Removed no-misused-capture-owner-stack rule
    • Cleaned up commented-out rule
  • Husky Hook: Removed .husky/post-merge hook (no longer needed)
  • Changeset: Added changeset for patch version bump

Motivation

Ensures React ESLint rules are properly configured according to the latest eslint-plugin-react-x recommendations and removes unused automation that is no longer required.

Summary by CodeRabbit

  • Updates

    • Updated React ESLint rules configuration to use improved rule set from eslint-plugin-react-x
  • Chores

    • Removed post-merge hook for conditional dependency installation
    • Updated internal documentation for commit message creation

- Update React ESLint rules configuration
- Remove unused post-merge hook
@mheob mheob added the eslint 🚨 Belongs to ESLint config label Oct 21, 2025
@changeset-bot
Copy link

changeset-bot bot commented Oct 21, 2025

🦋 Changeset detected

Latest commit: 58a868e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@mheob/eslint-config Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented Oct 21, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
eslint-config Ready Ready Preview Comment Oct 21, 2025 5:53am

💡 Enable Vercel Agent with $100 free credit for automated AI reviews

@coderabbitai
Copy link

coderabbitai bot commented Oct 21, 2025

Walkthrough

The PR updates React ESLint rules configuration by replacing core rule defaults with eslint-plugin-react-x rules, removes the post-merge git hook, updates commit creation documentation, and introduces a changeset file for a patch release to the @mheob/eslint-config package.

Changes

Cohort / File(s) Change Summary
Release Metadata
changeset/fix-react-rules.md
Adds new changeset entry for @mheob/eslint-config patch release documenting React ESLint rules configuration updates
ESLint Configuration
packages/eslint-config/src/configs/react.ts
Replaces core React rule defaults with eslint-plugin-react-x rules, including enabling jsx-no-comment-textnodes, jsx-no-duplicate-props, jsx-uses-react, and jsx-uses-vars at warn level; removes previously disabled rules and commented entries
Git Hooks
.husky/post-merge
Removes entire hook script that previously conditionally triggered pnpm install when pnpm-lock.yaml changed
Documentation
.claude/commands/create-commit.md
Renames section header and updates commit creation instructions with lowercase action phrases and full sentence formatting requirement

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • #249: Modifies the same React ESLint configuration file with updates to which React rules are enabled/disabled
  • #311: Alters plugin access and casting in the same packages/eslint-config/src/configs/react.ts file
  • #263: Updates React ESLint rule set and touches the react/no-leaked-conditional-rendering rule entry in the same config file

Suggested labels

bug, eslint

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "fix(eslint): update React rules and remove hook" directly corresponds to the main changes in this pull request. It captures the two primary substantive changes: the update to React ESLint rules configuration in packages/eslint-config/src/configs/react.ts and the removal of the .husky/post-merge hook. The title follows conventional commit format, is concise and readable, and provides clear information about what the PR accomplishes. While there is an auxiliary change to .claude/commands/create-commit.md, the title appropriately focuses on the core changes, which aligns with the expectation that titles need not cover every detail of the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-react-rules

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3e1b672 and 58a868e.

📒 Files selected for processing (4)
  • .changeset/fix-react-rules.md (1 hunks)
  • .claude/commands/create-commit.md (1 hunks)
  • .husky/post-merge (0 hunks)
  • packages/eslint-config/src/configs/react.ts (1 hunks)
💤 Files with no reviewable changes (1)
  • .husky/post-merge
🧰 Additional context used
📓 Path-based instructions (2)
.changeset/*.md

📄 CodeRabbit inference engine (CLAUDE.md)

Create a changeset for any user-facing change

Files:

  • .changeset/fix-react-rules.md
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/any-inside-generic-functions.mdc)

**/*.{ts,tsx}: When building generic functions in TypeScript, it is acceptable to use 'any' inside the function body if TypeScript cannot match runtime logic to type logic.
Outside of generic functions, use 'any' extremely sparingly in TypeScript code.

Unless explicitly required by the framework, do not use default exports in TypeScript files.

**/*.{ts,tsx}: Use TypeScript for all code. Prefer interfaces over types.
Follow the ESLint rules.
Use "function" keyword for pure functions.
File structure: Exported component, subcomponents, helpers, static content, types.
Avoid unnecessary curly braces in conditional statements.
For single-line statements in conditionals, omit curly braces.
Use concise, one-line syntax for simple conditional statements (e.g., if (condition) doSomething()).
Handle errors and edge cases at the beginning of functions.
Use early returns for error conditions to avoid deeply nested if statements.
Place the happy path last in the function for improved readability.
Avoid unnecessary else statements; use if-return pattern instead.
Use guard clauses to handle preconditions and invalid states early.
Implement proper error logging and user-friendly error messages.
Consider using custom error types or error factories for consistent error handling.
Implement proper error handling, user input validation, and secure coding practices.
Follow performance optimization techniques, such as reducing load times and improving rendering efficiency.
Provide clear and concise comments for complex logic.
Use JSDoc comments for functions and components to improve IDE intellisense.

**/*.{ts,tsx}: Proactively use discriminated unions to model data that can be in one of a few different shapes.
Use switch statements to handle the results of discriminated unions.
Use discriminated unions to prevent the 'bag of optionals' problem (i.e., avoid interfaces with multiple optional properties that can represent impossible states).

**/*.{ts,tsx}: Do not introduce new en...

Files:

  • packages/eslint-config/src/configs/react.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: claude-review
🔇 Additional comments (4)
.claude/commands/create-commit.md (1)

13-20: LGTM! Documentation improvements look good.

The clarifications to the task instructions and the addition of the note about omitting Claude as author improve the quality and appropriateness of generated commits.

packages/eslint-config/src/configs/react.ts (2)

169-169: LGTM! Comment accurately reflects the plugin source.

The updated comment correctly references eslint-plugin-react-x for these core rules.


170-173: Verify necessity of legacy JSX rules for target React version and configuration.

If using the new JSX transform from React 17, react/jsx-uses-react should be disabled by extending plugin:react/jsx-runtime. The react/jsx-uses-vars rule marks variables used in JSX as used and only has an effect when the no-unused-vars rule is enabled.

Please confirm:

  1. Does this config target React 17+ exclusively, or is it version-agnostic?
  2. Is no-unused-vars enabled in consuming configs?
  3. Should this config extend plugin:react/jsx-runtime for modern React?

If targeting React 17+, remove line 171 (react/jsx-uses-react) and ensure the config extends plugin:react/jsx-runtime. Line 172 (react/jsx-uses-vars) should be retained if no-unused-vars is enabled.

.changeset/fix-react-rules.md (1)

1-5: LGTM! Changeset is properly formatted.

The changeset correctly specifies a patch release for @mheob/eslint-config with an appropriate description of the React ESLint rules update.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud
Copy link

@claude
Copy link

claude bot commented Oct 21, 2025

PR Review: fix(eslint): update React rules and remove hook

Summary

This PR updates React ESLint rules to align with eslint-plugin-react-x recommendations and removes an unused post-merge hook. Overall, the changes are solid and well-motivated. I have a few observations and recommendations below.


✅ Code Quality & Best Practices

Positive aspects:

  • Correct plugin reference: Fixed the comment to reference eslint-plugin-react-x instead of eslint-plugin-react (packages/eslint-config/src/configs/react.ts:169)
  • Rule consolidation: Properly consolidated no-comment-textnodes into jsx-no-comment-textnodes to align with JSX-specific naming conventions
  • Re-enabled essential rules: The previously disabled JSX rules (jsx-no-duplicate-props, jsx-uses-react, jsx-uses-vars) are now properly enabled, which will catch common React mistakes
  • Cleanup: Removed unused post-merge hook (.husky/post-merge) - good maintenance practice
  • Changeset included: Proper version management with changeset for tracking this change

Recommendations:

  1. Changeset description: The changeset message is quite terse. Consider expanding it to mention the specific changes:

    ---
    '@mheob/eslint-config': patch
    ---
    
    Re-enable essential JSX ESLint rules and align with eslint-plugin-react-x recommendations.

    This helps consumers understand what changed in the release notes.

  2. create-commit.md changes: The changes to .claude/commands/create-commit.md improve the instructions, but they seem unrelated to the PR's main purpose. Consider:

    • Moving these to a separate PR focused on improving AI workflow commands, OR
    • Mentioning them explicitly in the PR description if they're intentionally included

🐛 Potential Issues

Medium Priority:

  1. TypeGen file out of sync: The file packages/eslint-config/src/typegen.d.ts still contains type definitions for react/no-misused-capture-owner-stack (line 3058), which was removed in this PR. This suggests:

    • The typegen file is auto-generated and needs regeneration
    • Or it's manually maintained and needs updating

    Action needed: Run the type generation script after merging, or update typegen.d.ts manually to remove the obsolete rule definition.

Low Priority:
2. Rule removal justification: The removal of react/no-misused-capture-owner-stack isn't explained. This could be because:

  • The rule was removed from eslint-plugin-react-x
  • It's causing false positives
  • It's been superseded by another rule

Consider adding a brief comment in the commit message or PR description explaining why this rule was removed.


⚡ Performance Considerations

  • No performance impact: The changes are configuration-only and won't affect runtime performance
  • Build time: Re-enabling linting rules may slightly increase lint time, but the impact is negligible
  • Type checking: If typegen.d.ts is auto-generated, ensure the generation process is run as part of the build pipeline

🔒 Security Concerns

No security issues identified. The changes are improving code quality by:

  • Catching duplicate JSX props that could mask bugs
  • Preventing commented-out code from being rendered in the DOM
  • Ensuring variables used in JSX are properly marked as "used"

These rules help prevent subtle bugs that could have security implications in React applications.


📋 Additional Recommendations

  1. Testing: After merging, test the config on an actual React project to ensure:

    • The re-enabled rules work as expected
    • No unexpected lint errors appear in previously passing code
    • The removed no-misused-capture-owner-stack rule doesn't cause issues
  2. Documentation: Consider updating the package README or migration guide if users might see new lint errors after upgrading due to the re-enabled rules

  3. Post-merge hook removal: Verify that the pnpm install automation is handled elsewhere (e.g., in CI/CD or developer documentation) since the post-merge hook is being removed


Verdict: ✅ Approve with minor suggestions

The core changes are well-reasoned and improve the ESLint configuration. The only blocker would be ensuring the typegen.d.ts file is regenerated/updated before or after merge to maintain consistency.

Suggested next steps:

  1. Regenerate or update typegen.d.ts to remove obsolete rule type
  2. Consider enhancing the changeset description
  3. Clarify whether .claude/commands/create-commit.md changes are intentional

@kodiakhq kodiakhq bot merged commit bb18a7d into main Oct 21, 2025
10 checks passed
@kodiakhq kodiakhq bot deleted the fix-react-rules branch October 21, 2025 06:00
kodiakhq bot pushed a commit that referenced this pull request Oct 21, 2025
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated.


# Releases
## @mheob/[email protected]

### Patch Changes

-   [#315](#315) ([@mheob](https://github.com/mheob)): Update React ESLint rules configuration.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

eslint 🚨 Belongs to ESLint config

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants