-
-
Couldn't load subscription status.
- Fork 638
Fix prettier formatting issues from PR #1769 #1774
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
Conversation
- Fix formatting in CHANGELOG.md - Fix formatting in CODING_AGENTS.md - Fix formatting in docs files - Fix formatting in CSS module test fixtures These formatting issues were introduced in the merged PR #1769 and are causing CI failures.
WalkthroughThis PR primarily updates documentation formatting, adjusts a migration example for react_component props structure, expands v15 release notes around hydration behavior, and adds trailing newlines to two fixture files. No code logic or public API changes are introduced. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant B as Browser
participant R as ReactOnRails
participant S as Redux Store
participant C as React Components
U->>B: Navigate to page
B->>R: Load page scripts (no defer required)
rect rgba(230,245,255,0.5)
note over B,R: Streaming HTML begins
R-->>B: Initial HTML stream
R->>R: reactOnRailsPageLoaded() (async)
R-->>B: Promise pending
end
alt force_load true (default)
R->>S: Initialize stores
else force_load false (per-store override)
R->>S: Defer store init until needed
end
R->>C: Hydrate components
R-->>B: Promise resolves when hydration complete
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing Touches
🧪 Generate unit tests
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. Comment |
Code ReviewSummaryThis PR addresses formatting issues from PR #1769 that were causing CI failures. The changes are purely cosmetic and focus on fixing prettier/linting violations. ✅ Code Quality & Best Practices
✅ Potential Issues
✅ Performance Considerations
✅ Security Concerns
✅ Test Coverage
Recommendations
Overall AssessmentThis is a straightforward formatting fix that addresses CI failures. The changes are minimal, safe, and necessary for maintaining code quality standards. Recommendation: ✅ Approve and merge |
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: 0
🧹 Nitpick comments (2)
docs/release-notes/15.0.0.md (2)
61-63: Tighten wording and branding for clarityMinor copy edits to improve readability and use the product name vs. package name.
- - The previous need for deferring scripts to prevent race conditions has been eliminated due to improved hydration handling. Making scripts not defer is critical to execute the hydration scripts early before the page is fully loaded. + - The prior need to defer scripts to prevent race conditions has been eliminated by improved hydration handling. Do not defer generated component packs; this ensures hydration scripts execute early, before the page is fully loaded. - - The `force_load` configuration makes `react-on-rails` hydrate components immediately as soon as their server-rendered HTML reaches the client, without waiting for the full page load. + - The `force_load` configuration makes React on Rails hydrate components immediately as soon as their server-rendered HTML reaches the client, without waiting for the full page load.
68-82: Note: call site must be in an async contextAdd a one-liner so readers don’t trip on environments without top‑level await.
- If you manually call this function to ensure components are hydrated (e.g., with async script loading), you must now await the promise it returns: + - Ensure this call runs in an async context (or wrap in an async IIFE) if your environment doesn’t support top‑level `await`.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
CHANGELOG.md(2 hunks)CODING_AGENTS.md(1 hunks)docs/additional-details/migrating-from-react-rails.md(1 hunks)docs/getting-started.md(1 hunks)docs/guides/streaming-server-rendering.md(2 hunks)docs/guides/upgrading-react-on-rails.md(4 hunks)docs/release-notes/15.0.0.md(2 hunks)spec/dummy/spec/fixtures/automated_packs_generation/components/ComponentWithCSSModule/ror_components/ComponentWithCSSModule.jsx(1 hunks)spec/dummy/spec/fixtures/automated_packs_generation/components/ComponentWithCSSModule/ror_components/ComponentWithCSSModule.module.css(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-04-26T21:55:55.874Z
Learnt from: alexeyr-ci2
PR: shakacode/react_on_rails#1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.
Applied to files:
docs/additional-details/migrating-from-react-rails.mddocs/guides/upgrading-react-on-rails.md
📚 Learning: 2025-02-13T16:50:47.848Z
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: node_package/src/clientStartup.ts:18-21
Timestamp: 2025-02-13T16:50:47.848Z
Learning: In the react_on_rails module, the `reactOnRailsPageUnloaded` function in clientStartup.ts is intentionally kept private as it's only used internally as a callback for `onPageUnloaded`.
Applied to files:
docs/release-notes/15.0.0.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). (5)
- GitHub Check: dummy-app-integration-tests (newest)
- GitHub Check: dummy-app-integration-tests (oldest)
- GitHub Check: rspec-package-tests (newest)
- GitHub Check: rspec-package-tests (oldest)
- GitHub Check: build
🔇 Additional comments (9)
docs/getting-started.md (1)
41-41: Prettier spacing after heading — looks goodBlank line after “3. Start the app:” fixes Markdown list rendering.
CODING_AGENTS.md (1)
262-262: Consistent Markdown spacingInserted blank lines after numbered subheadings improve formatting and satisfy Prettier.
Also applies to: 268-268, 273-273
spec/dummy/spec/fixtures/automated_packs_generation/components/ComponentWithCSSModule/ror_components/ComponentWithCSSModule.module.css (1)
9-9: Add trailing newlineEOF newline aligns with linters/CI expectations.
spec/dummy/spec/fixtures/automated_packs_generation/components/ComponentWithCSSModule/ror_components/ComponentWithCSSModule.jsx (1)
12-12: Add trailing newlineGood fix for prettier/CI.
docs/guides/streaming-server-rendering.md (1)
122-122: Prettier-compliant blank linesSpacing tweaks improve list and section rendering; no content changes.
Also applies to: 168-168, 174-174
CHANGELOG.md (1)
445-445: Whitespace-only cleanupExtra blank lines improve readability and satisfy formatting checks.
Also applies to: 1163-1163, 1169-1169
docs/additional-details/migrating-from-react-rails.md (1)
6-6: Correct helper signature exampleSwitch to two-arg form with props: key is accurate and consistent with current API.
Also applies to: 14-14, 18-18, 27-30
docs/guides/upgrading-react-on-rails.md (1)
240-240: Formatting-only adjustmentsRemoval of redundant subheading and spacing changes improve structure; content unchanged.
Also applies to: 253-253, 266-266, 296-296
docs/release-notes/15.0.0.md (1)
60-60: LGTM: formatting fix unblocks Prettier/Markdown nestingThe inserted blank line correctly separates paragraphs from the nested list and should satisfy the formatter.
Code Review - PR #1774: Fix prettier formatting issues from PR #1769Overall Assessment ✅This is a clean-up PR that addresses formatting inconsistencies introduced in PR #1769. The changes are primarily cosmetic and focus on improving code consistency and readability. Code Quality and Best Practices ✅Strengths:
Areas of Excellence:
Potential Issues - None Identified ✅No functional bugs or issues detected. All changes are cosmetic formatting improvements. Performance Considerations ✅No performance impact - these are documentation and formatting changes only. Security Concerns ✅No security implications as this is purely formatting changes. Test Coverage 📝Current Status: The PR checklist shows unchecked items for tests and documentation updates. Recommendation: For a formatting-only PR like this:
Specific Changes Reviewed:
Recommendations:
ConclusionThis is a well-executed formatting fix that maintains code quality standards. The changes align with the repository's formatting conventions and successfully address the CI failures mentioned in the PR description. Status: APPROVED ✅ |
|
update docs on how to lint, prettier |
These formatting issues were introduced in the merged PR #1769 and are causing CI failures.
Summary
Remove this paragraph and provide a general description of the code changes in your pull
request... were there any bugs you had fixed? If so, mention them. If
these bugs have open GitHub issues, be sure to tag them here as well,
to keep the conversation linked together.
Pull Request checklist
Remove this line after checking all the items here. If the item is not applicable to the PR, both check it out and wrap it by
~.Add the CHANGELOG entry at the top of the file.
Other Information
Remove this paragraph and mention any other important and relevant information such as benchmarks.
This change is
Summary by CodeRabbit
Documentation
Style