Skip to content

Conversation

@justin808
Copy link
Member

@justin808 justin808 commented Nov 9, 2025

Summary

Adds comprehensive documentation for the custom ESLint rules directory, specifically documenting the no-use-client-in-server-files rule that enforces proper usage of server components in React Server Components architecture.

What's Included

  • Why the rule exists (Shakapacker 9.3.0+ compatibility)
  • Clear examples of correct and incorrect usage
  • Auto-fix capability documentation
  • Testing instructions
  • Guidance for adding new custom rules in the future

This change is Reviewable

Summary by CodeRabbit

  • Documentation
    • Added comprehensive documentation for a custom ESLint rule, including usage examples, configuration guidelines, auto-fix capabilities, and testing procedures.

Documents the no-use-client-in-server-files rule, including why it exists (Shakapacker 9.3.0+ compatibility), usage examples, auto-fix capability, and guidance for adding new custom rules.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 9, 2025

Walkthrough

A new documentation file is added to explain a custom ESLint rule that prevents 'use client' directives in server-only files (.server.tsx/.server.ts), including configuration, examples, and guidance for adding additional custom rules.

Changes

Cohort / File(s) Summary
ESLint Rules Documentation
eslint-rules/README.md
New file documenting the no-use-client-in-server-files custom ESLint rule, including purpose, rationale for React Server Components, usage examples, auto-fix behavior, configuration integration, testing instructions, and guidelines for adding future rules.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Documentation addition only; no functional code or logic changes to evaluate

Suggested reviewers

  • AbanoubGhadban

Poem

🐰 A rule to keep the client from the server's sacred ground,
No "use client" in .server files, let's make that sound!
With docs so clear and examples bright,
Our linting journey feels just right! ✨

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 directly and clearly describes the main change: adding documentation for ESLint custom rules, which aligns perfectly with the changeset that adds a README documenting the no-use-client-in-server-files rule.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch justin808/troy

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1e68434 and 8f50ed7.

📒 Files selected for processing (1)
  • eslint-rules/README.md (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-16T08:01:11.146Z
Learnt from: justin808
Repo: shakacode/react_on_rails PR: 1770
File: lib/generators/react_on_rails/templates/base/base/app/javascript/src/HelloWorld/ror_components/HelloWorld.client.jsx:2-2
Timestamp: 2025-09-16T08:01:11.146Z
Learning: React on Rails uses webpack CSS Modules configuration with namedExports: true, which requires the import syntax `import * as style from './file.module.css'` rather than the default export pattern. This configuration enables better tree shaking and bundle size optimization for CSS modules.

Applied to files:

  • eslint-rules/README.md
📚 Learning: 2025-02-13T19:09:15.991Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: node_package/src/RSCWebpackLoader.ts:0-0
Timestamp: 2025-02-13T19:09:15.991Z
Learning: In React Server Components webpack loader, using `new Function('return import("react-server-dom-webpack/node-loader")')()` is necessary as a workaround to bypass TypeScript compilation issues with direct dynamic imports.

Applied to files:

  • eslint-rules/README.md
⏰ 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: markdown-link-check
🔇 Additional comments (6)
eslint-rules/README.md (6)

7-21: ✅ Technical explanation is accurate and well-motivated.

The rationale for the no-use-client-in-server-files rule is clearly explained, with proper context about React Server Components architecture and webpack's resolve.conditionNames behavior. The listed React APIs are correctly identified as client-only exports.


22-60: ✅ Examples are clear and comprehensive.

The provided examples effectively illustrate both incorrect and correct usage patterns, with good visual distinction. The examples cover server-only files, client-only files, and generic files appropriately.


95-110: ✅ Testing and new rules guidance is actionable.

The testing instructions are clear, and the step-by-step guidance for adding new rules is well-structured and easy to follow. Developers will have a clear path for extending the custom rules in the future.


1-5: ✅ Documentation structure and organization is well-executed.

The README follows a logical progression from rule explanation through examples to testing and extension guidance. The markdown formatting is clean and professional, making it easy for developers to navigate.


1-110: Based on my verification, the configuration snippet in the README accurately matches the actual eslint.config.ts implementation. The rule is correctly registered under the 'react-on-rails' plugin, scoped to .server.ts and .server.tsx files, with the error level set appropriately. The technical documentation about React Server Components, webpack's resolve.conditionNames, and conditional exports is also accurate.


72-74: Links referenced cannot be verified; recommend manual confirmation.

The review comment references specific GitHub resources (Shakapacker issue #805, React on Rails PR #1896, and commit 86979dc) but web searches did not return these specific resources. While this doesn't prove the links are incorrect, they should be manually verified to ensure they remain accurate and accessible.


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.

@claude
Copy link

claude bot commented Nov 9, 2025

Pull Request Review

Overview

This PR adds comprehensive documentation for the custom ESLint rules directory, specifically documenting the no-use-client-in-server-files rule. This is a documentation-only change that adds clarity for developers working with React Server Components.

✅ Strengths

  1. Clear Documentation Structure: The README follows a logical flow - what the rule does, why it exists, examples, configuration, and testing.

  2. Excellent Context: The "Why This Rule Exists" section provides critical background about the Shakapacker 9.3.0+ compatibility issue and React's conditional exports. This helps developers understand the reasoning, not just the rule.

  3. Helpful Examples: The ❌/✅ pattern for incorrect/correct usage is very clear and actionable.

  4. Auto-fix Documentation: Documents that the rule is fixable with --fix, which is important for developer experience.

  5. Comprehensive Links: References to related issues, PRs, and commits provide excellent traceability.

  6. Future-Proofing: The "Adding New Custom Rules" section helps maintainers extend the system.

📝 Suggestions for Improvement

1. Minor Inconsistency in File Extension Coverage

The README mentions both .server.tsx and .server.ts files in the rule description, but the title and initial description only mention .server.tsx. Consider updating line 8 to be more explicit:

### `no-use-client-in-server-files`

Prevents the `'use client'` directive from being used in `.server.tsx` and `.server.ts` files.

This matches the actual implementation in no-use-client-in-server-files.cjs:36.

2. Testing Command Could Be More Specific

Line 98 shows:

node eslint-rules/no-use-client-in-server-files.test.cjs

Consider adding context about expected output:

# Run tests for the custom rules (should exit with code 0 if all tests pass)
node eslint-rules/no-use-client-in-server-files.test.cjs

3. Configuration Section Reference

Line 75 states "This rule is automatically enabled in the React on Rails ESLint configuration at the error level." It might be helpful to specify that this only applies to .server.ts and .server.tsx files (as seen in eslint.config.ts), to avoid confusion.

4. URL Update Consideration

Line 23 in no-use-client-in-server-files.cjs references:

url: 'https://github.com/shakacode/react_on_rails/pull/1919',

Consider updating this to point to the new README once this PR is merged, as it will be the canonical documentation. Could be done in a follow-up PR.

🔍 Code Quality Review

Rule Implementation (no-use-client-in-server-files.cjs):

  • ✅ Proper ESLint rule structure with meta, messages, and schema
  • ✅ Auto-fix implementation is safe and removes only the directive
  • ✅ Regex pattern handles both single and double quotes with backreference
  • ✅ Handles optional semicolon and whitespace variations
  • ✅ Good backward compatibility with both context.filename and context.getFilename()

Test Coverage (no-use-client-in-server-files.test.cjs):

  • ✅ Comprehensive valid cases (server files without directive, client files with directive)
  • ✅ Tests both .server.tsx and .server.ts extensions
  • ✅ Tests auto-fix output for multiple quote styles
  • ✅ Tests edge cases like leading whitespace and missing semicolons
  • ✅ Tests that the rule doesn't trigger for non-server files

🎯 Best Practices Alignment

Per CLAUDE.md:

  • ✅ Documentation follows project conventions
  • ✅ No CHANGELOG entry needed (this is a documentation-only change per CLAUDE.md guidelines)
  • ⚠️ Note: When merging, ensure files end with newlines and run bundle exec rubocop (though this PR only adds markdown, so mainly applies to future rule additions)

🔒 Security, Performance, and Bugs

  • ✅ No security concerns - documentation only
  • ✅ No performance impact - documentation only
  • ✅ No bugs identified in the existing rule implementation
  • ✅ Regex pattern is safe and specific (anchored with ^, uses backreference for quote matching)

📊 Overall Assessment

This is a high-quality documentation PR that significantly improves the discoverability and understanding of the custom ESLint rules system. The documentation is well-written, comprehensive, and provides excellent context for both current and future developers.

Recommendation: ✅ Approve with minor suggestions

The suggestions above are minor improvements and not blockers. The PR is ready to merge as-is, but would benefit from the small enhancements noted.


Great work on documenting this important rule! The context about Shakapacker 9.3.0+ and React Server Components will save developers significant debugging time.

@justin808 justin808 merged commit 1d95f18 into master Nov 9, 2025
14 checks passed
@justin808 justin808 deleted the justin808/troy branch November 9, 2025 03:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants