-
-
Notifications
You must be signed in to change notification settings - Fork 637
Make RSC tests await act #1734
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?
Make RSC tests await act #1734
Conversation
WalkthroughThe updates introduce the Changes
Sequence Diagram(s)sequenceDiagram
participant TestFile
participant ESLint
participant TestingLibraryPlugin
TestFile->>ESLint: Lint test files (*.test.{js,jsx,ts,tsx})
ESLint->>TestingLibraryPlugin: Apply Testing Library DOM rules
TestingLibraryPlugin-->>ESLint: Return linting results
ESLint-->>TestFile: Report linting results
sequenceDiagram
participant Test
participant RenderFunction
participant Component
Test->>RenderFunction: await render()
RenderFunction->>Component: Render asynchronously
Component-->>RenderFunction: Rendering complete (Promise resolved)
RenderFunction-->>Test: Continue test execution
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
CodeRabbit Configuration File (
|
9ec8d5f
to
11d72c2
Compare
e5700dc
to
a74f298
Compare
a74f298
to
82cf696
Compare
bd4c797
to
89593b1
Compare
1b743d8
to
40b5df9
Compare
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 (1)
docs/react-on-rails-pro/react-on-rails-pro.md (1)
19-21
: Replace bare URL with markdown-formatted link to satisfy MD034
markdownlint
is flagging the bare URL on line 21. Consistently using markdown links keeps the docs lint-clean and matches the surrounding style.-See https://www.shakacode.com/react-on-rails-pro/docs/. +See the full documentation [here](https://www.shakacode.com/react-on-rails-pro/docs/).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (7)
docs/react-on-rails-pro/react-on-rails-pro.md
(1 hunks)docs/react-on-rails-pro/react-server-components.md
(1 hunks)eslint.config.ts
(2 hunks)node_package/tests/SuspenseHydration.test.tsx
(2 hunks)node_package/tests/registerServerComponent.client.test.jsx
(5 hunks)package.json
(2 hunks)script/convert
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/react-on-rails-pro/react-server-components.md
🚧 Files skipped from review as they are similar to previous changes (5)
- package.json
- eslint.config.ts
- node_package/tests/registerServerComponent.client.test.jsx
- script/convert
- node_package/tests/SuspenseHydration.test.tsx
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: alexeyr-ci
PR: shakacode/react_on_rails#1644
File: node_package/src/ReactOnRailsRSC.ts:87-87
Timestamp: 2024-12-12T13:07:09.929Z
Learning: When handling errors in 'node_package/src/ReactOnRailsRSC.ts', include the error stack in error messages in development and test environments to aid debugging.
Learnt from: alexeyr-ci
PR: shakacode/react_on_rails#1687
File: spec/dummy/package.json:0-0
Timestamp: 2025-01-23T18:20:45.824Z
Learning: When adding or updating dependencies in spec/dummy/package.json, maintain version consistency with other package.json files in the codebase to avoid potential version conflicts.
docs/react-on-rails-pro/react-on-rails-pro.md (10)
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.
Learnt from: Romex91
PR: shakacode/react_on_rails#1697
File: package-scripts.yml:28-28
Timestamp: 2025-02-12T16:38:06.537Z
Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: lib/react_on_rails/helper.rb:190-197
Timestamp: 2025-02-18T13:08:01.477Z
Learning: RSC support validation in React on Rails Pro is handled through a chain of validations:
1. Pro version check in `run_stream_inside_fiber`
2. RSC support check during pack generation via `ReactOnRailsPro.configuration.enable_rsc_support`
3. RSC support validation during component registration
This makes additional validation in the helper methods unnecessary.
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: lib/react_on_rails/helper.rb:190-197
Timestamp: 2025-02-18T13:08:01.477Z
Learning: RSC support validation is handled in deeper level calls of the React on Rails Pro codebase, so it doesn't need to be validated again in the `rsc_payload_react_component` helper method.
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`.
Learnt from: theforestvn88
PR: shakacode/react_on_rails#1620
File: spec/dummy/client/app/startup/HelloTurboStream.jsx:3-3
Timestamp: 2024-10-08T20:53:47.076Z
Learning: The `RailsContext` import in `spec/dummy/client/app/startup/HelloTurboStream.jsx` is used later in the project, as clarified by the user theforestvn88.
Learnt from: theforestvn88
PR: shakacode/react_on_rails#1620
File: spec/dummy/client/app/startup/HelloTurboStream.jsx:3-3
Timestamp: 2024-07-27T10:08:35.868Z
Learning: The `RailsContext` import in `spec/dummy/client/app/startup/HelloTurboStream.jsx` is used later in the project, as clarified by the user theforestvn88.
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1644
File: node_package/src/turbolinksUtils.ts:34-36
Timestamp: 2025-02-13T16:50:26.861Z
Learning: In React on Rails, when checking for Turbolinks version 5 using `turbolinksVersion5()`, always ensure `Turbolinks` exists first by checking `turbolinksInstalled()` to prevent TypeError when accessing properties.
Learnt from: AbanoubGhadban
PR: shakacode/react_on_rails#1696
File: node_package/src/RSCPayloadContainer.ts:0-0
Timestamp: 2025-04-09T12:56:10.756Z
Learning: In the react_on_rails codebase, RSC payloads are already stringified using `JSON.stringify()` before being processed by the `escapeScript` function, which handles escaping of special characters. The function only needs to handle specific HTML markers like comments and closing script tags.
Learnt from: alexeyr-ci
PR: shakacode/react_on_rails#1644
File: node_package/src/ReactOnRailsRSC.ts:87-87
Timestamp: 2024-12-12T13:07:09.929Z
Learning: When handling errors in 'node_package/src/ReactOnRailsRSC.ts', include the error stack in error messages in development and test environments to aid debugging.
🪛 markdownlint-cli2 (0.17.2)
docs/react-on-rails-pro/react-on-rails-pro.md
21-21: Bare URL used
null
(MD034, no-bare-urls)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: dummy-app-integration-tests (oldest)
- GitHub Check: dummy-app-integration-tests (newest)
- GitHub Check: rspec-package-tests (newest)
- GitHub Check: rspec-package-tests (oldest)
- GitHub Check: build
🔇 Additional comments (1)
docs/react-on-rails-pro/react-on-rails-pro.md (1)
23-24
: Spacing tweak looks goodAdding the blank line after the “Pro: React Server Components” header improves readability and keeps heading spacing consistent across the doc.
Summary
Tests in
node_package/tests/RSCClientRoot.test.jsx
were callingact
withoutawait
, leading to these warnings:and
Pull Request checklist
Add/update test to cover these changes
[ ] Update documentation[ ] Update CHANGELOG file(unless implementation ends up needing to change)Should add https://github.com/testing-library/eslint-plugin-testing-library and maybe https://github.com/testing-library/eslint-plugin-jest-dom to prevent the issue repeating.
This change is
Summary by CodeRabbit