Add Support for Async Render Functions Returning React Components#1720
Conversation
WalkthroughThe update introduces a new documentation guide on render functions in React on Rails and adjusts multiple source modules to improve type flexibility, error handling, and asynchronous processing during server rendering. Changes include updates to method signatures, new helper functions, refined type definitions, and expanded tests. Additionally, modifications to streaming error reporting and ESLint configuration help streamline rendering workflows. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant S as Streaming Renderer
participant P as Promise Resolver
participant R as ReactDOMServer
participant E as Error Handler
C->>S: Request streaming render
S->>P: Resolve reactRenderingResult
P-->>S: Return resolved value (React element or string)
alt Resolved value is a valid React element
S->>R: Render element via processReactElement
R-->>S: Return rendered HTML
S-->>C: Stream rendered HTML
else Resolved value is an error or string
S->>E: Invoke reportError/sendErrorHtml
E-->>S: Provide error HTML response
S-->>C: Stream error HTML
end
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
docs/javascript/render-functions.md (2)
101-110: Remove trailing colons in headingsThe headings "When to use:" and similar headings throughout the document have trailing punctuation which should be removed according to markdown best practices.
-#### When to use: +#### When to use🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
101-101: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
107-107: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
147-149: Consider using consistent formatting for requirementsThe requirements section uses hyphens that could be misinterpreted as "and" vs "or". Consider using consistent formatting or explicit conjunctions.
-#### Requirements: +#### Requirements -The render function MUST return an object -The object MUST include a `componentHtml` key -All other keys are optional and can be accessed in your Rails view +The render function MUST return an object. +The object MUST include a `componentHtml` key. +All other keys are optional and can be accessed in your Rails view.🧰 Tools
🪛 LanguageTool
[misspelling] ~147-~147: Did you mean “and”?
Context: ...nts: - The render function MUST return an object - The object MUST include a `com...(AN_AND)
node_package/src/serverRenderReactComponent.ts (1)
47-56: Log the original error details for better debugging.This block catches rendering errors and logs a message. Logging the error’s stack (or relevant details) can be invaluable for diagnosing issues unrelated to double render calls.
Add the original error details alongside the custom message:
} catch (error) { console.error( `Invalid call to renderToString. Possibly you have a renderFunction, a function that already calls renderToString, that takes one parameter. You need to add an extra unused parameter to identify this function as a renderFunction and not a simple React Function Component.`, + error ); throw error; }node_package/tests/streamServerRenderedReactComponent.test.jsx (1)
63-97: Use ofcomponentTypeadds clarity to test coverage.This switch-based approach for registering different component types effectively tests all sync/async error and success paths. Consider refactoring repeated error-throwing stubs to reduce duplication, but overall functionality looks good.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
docs/javascript/render-functions.md(1 hunks)node_package/src/ReactOnRailsRSC.ts(2 hunks)node_package/src/createReactOutput.ts(2 hunks)node_package/src/handleError.ts(1 hunks)node_package/src/isServerRenderResult.ts(2 hunks)node_package/src/serverRenderReactComponent.ts(2 hunks)node_package/src/streamServerRenderedReactComponent.ts(4 hunks)node_package/src/types/index.ts(1 hunks)node_package/tests/serverRenderReactComponent.test.js(0 hunks)node_package/tests/serverRenderReactComponent.test.ts(1 hunks)node_package/tests/streamServerRenderedReactComponent.test.jsx(6 hunks)
💤 Files with no reviewable changes (1)
- node_package/tests/serverRenderReactComponent.test.js
🧰 Additional context used
🧬 Code Definitions (5)
node_package/src/ReactOnRailsRSC.ts (2)
node_package/src/types/index.ts (1)
RSCRenderParams(143-145)node_package/src/loadReactClientManifest.ts (1)
loadReactClientManifest(7-24)
node_package/src/isServerRenderResult.ts (1)
node_package/src/types/index.ts (3)
CreateReactOutputResult(105-105)RenderFunctionResult(102-102)ServerRenderResult(106-106)
node_package/src/streamServerRenderedReactComponent.ts (3)
node_package/src/types/index.ts (1)
RenderParams(137-141)node_package/src/ReactOnRails.client.ts (1)
handleError(302-306)node_package/src/serverRenderUtils.ts (1)
convertToError(20-22)
node_package/src/createReactOutput.ts (1)
node_package/src/isServerRenderResult.ts (2)
isServerRenderHash(3-12)isPromise(14-18)
node_package/tests/serverRenderReactComponent.test.ts (1)
node_package/src/types/index.ts (6)
RenderParams(137-141)RenderResult(163-169)RenderFunction(101-101)RailsContext(17-35)ServerRenderResult(106-106)RenderFunctionResult(102-102)
🪛 GitHub Actions: Lint JS and Ruby
node_package/tests/serverRenderReactComponent.test.ts
[error] 1-1: Parsing error: /home/runner/work/react_on_rails/react_on_rails/node_package/tests/serverRenderReactComponent.test.ts was not found by the project service. Consider either including it in the tsconfig.json or including it in allowDefaultProject.
🪛 LanguageTool
docs/javascript/render-functions.md
[uncategorized] ~79-~79: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...bjects without a renderedHtml property and they will still work. ## Ruby Helper F...
(COMMA_COMPOUND_SENTENCE)
[style] ~136-~136: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...le HTML strings in an object - When you need to insert rendered content in different pa...
(REP_NEED_TO_VB)
[misspelling] ~147-~147: Did you mean “and”?
Context: ...nts: - The render function MUST return an object - The object MUST include a `com...
(AN_AND)
[uncategorized] ~155-~155: Loose punctuation mark.
Context: ...es errors - either use JSX or wrap in a { renderedHtml: '...' } object. 3. **Async Functions Work wit...
(UNLIKELY_OPENING_PUNCTUATION)
🪛 markdownlint-cli2 (0.17.2)
docs/javascript/render-functions.md
101-101: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
107-107: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
133-133: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
140-140: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
145-145: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
🔇 Additional comments (49)
node_package/src/handleError.ts (2)
63-66: Good defensive programming to handle undefined renderToStringThis change adds a check to ensure
ReactDOMServer.renderToStringis a function before attempting to call it. This prevents potential runtime errors ifrenderToStringis unavailable in certain environments or scenarios.
63-66: Good defensive programming technique!This added check ensures the code handles situations where
ReactDOMServer.renderToStringmight not be available as a function, preventing potential runtime errors. This makes the error handling more robust across different environments.node_package/src/createReactOutput.ts (8)
5-23: Good extraction of render function result handling logicThe new
createReactElementFromRenderFunctionResultfunction cleanly encapsulates the logic for processing render function results. The warning about returning JSX directly helps guide developers toward the proper pattern that will work with React Hooks in v13.
70-74: Clean up of isServerRenderHash handlingThe code now cleanly handles the server render hash case without type assertions, improving readability and type safety.
76-89: Enhanced promise handling for render function resultsThe promise handling now properly differentiates between different result types (strings, functions, or other values) and processes each appropriately. This makes the function more robust when handling asynchronous render function results.
91-91: Simplified code with extracted functionUsing the new
createReactElementFromRenderFunctionResultfunction here simplifies the main flow and improves maintainability.
5-23: Well-structured helper function with good warning messageThe new
createReactElementFromRenderFunctionResultfunction provides a clean way to handle different return types from render functions. The warning message is especially valuable as it guides developers toward best practices for React Hooks compatibility in future versions.
70-74: Clean type handlingThe simplified check for
isServerRenderHashimproves code readability by removing unnecessary type assertions.
76-89: Enhanced promise handling for async render functionsThis improved logic for handling promises correctly processes different types of resolved values:
- Strings are returned directly
- Functions are processed using the new helper function
- Other values are stringified
This enhancement provides robust support for asynchronous render functions, which is the core objective of this PR.
91-91: Consistent use of helper functionUsing the new helper function here maintains consistency throughout the codebase.
node_package/src/ReactOnRailsRSC.ts (4)
24-26: Function signature expanded to support async render functionsThe function now accepts
reactRenderingResultof typeReactElement | Promise<ReactElement | string>instead of justReactElement. This enhancement allows the component to handle both synchronous and asynchronous rendering results.
37-38: Optimized parallel loading of manifest and rendering resultUsing
Promise.allto concurrently load the React client manifest and resolve the rendering result is an effective optimization. This approach avoids sequential processing, potentially improving performance.
24-26: Improved function signature for async supportUpdating the parameter type from
ReactElementtoReactElement | Promise<ReactElement | string>broadens the range of valid inputs, making the function more flexible and aligned with modern React patterns involving async operations.
37-38: Performance optimization with Promise.allUsing
Promise.allto concurrently load the React client manifest and resolve the rendering result is more efficient than sequential processing. This enhances performance when dealing with async operations, especially if either operation takes significant time.docs/javascript/render-functions.md (4)
1-3: Excellent documentation title and introductionThe title clearly indicates this is a usage guide for render functions in React on Rails, which helps users immediately understand the document's purpose.
5-69: Comprehensive coverage of render function return typesThe documentation thoroughly covers all possible return types from render functions with clear examples for each case. This is extremely helpful for developers to understand the options available to them.
71-80: Valuable information about limitationsHighlighting the important limitations observed in tests provides crucial information for developers to avoid common pitfalls when using render functions.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~79-~79: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...bjects without arenderedHtmlproperty and they will still work. ## Ruby Helper F...(COMMA_COMPOUND_SENTENCE)
161-290: Excellent examples with both JavaScript and Ruby codeThe comprehensive examples showing both the JavaScript render functions and their corresponding Ruby usage provide a complete picture for developers. This dual-language approach significantly improves the documentation's utility.
node_package/src/serverRenderReactComponent.ts (2)
59-59: Signature update for async returns looks good.Expanding
Promise<string>toPromise<string | ReactElement>aligns with supporting asynchronous React element creation.
71-75:Details
✅ Verification successful
Confirm promise type assumptions.
This logic assumes promiseResult is always a string or a valid ReactElement. If there’s any scenario where it could return a different type (e.g., null, number, or object), this code would fail. Ensure upstream calls respect this contract.
Check all call sites to confirm only strings or
ReactElements are ever returned.
🏁 Script executed:
#!/bin/bash rg -A 3 'processPromise'Length of output: 710
Confirmed promise type contract.
After verifying with the call sites and type definitions, it’s clear that the contract enforced by
processPromiseguarantees that the resolved value is either a string or a valid ReactElement. The upstream code ensures that no other types (e.g., null, number, or object) will be returned, so the logic in these lines is safe.
- File:
node_package/src/serverRenderReactComponent.ts(Lines 71-75)- Verification: The function signature for
processPromiseenforces a return type ofPromise<string> | string, ensuring promiseResult is either a string or a ReactElement.- Action: No further changes are required unless new types are introduced upstream.
node_package/src/isServerRenderResult.ts (2)
1-5: Broadening function signatures for added flexibility.Including
RenderFunctionResultensures the type guard can handle all new async/sync return possibilities without regressions.
15-15: Promise type signature update is consistent.Allowing
RenderFunctionResultinisPromisematches the broader support for async returns within render functions.node_package/src/types/index.ts (4)
45-48: Introduction ofServerRenderHashRenderedHtmlis a clear approach.This structure is well-defined and helps clarify the presence of a
componentHtmlfield with optional extra keys.
51-51: Union type onrenderedHtmlbroadens support.Enabling either a string or the new
ServerRenderHashRenderedHtmltype aligns with supporting complex SSR outputs.
57-57: ExtendedCreateReactOutputResulttype for async React elements.Including
Promise<string | ReactElement>accommodates asynchronous rendering seamlessly.
59-62:RenderFunctionResultnow correctly accounts for async/sync returns.Supporting promises of string,
ServerRenderHashRenderedHtml, orReactComponentunifies multiple SSR return paths into one flexible type.node_package/tests/streamServerRenderedReactComponent.test.jsx (13)
118-118: Using.once('end')prevents accidental double invocation.This is a safer way to handle the stream's end event.
137-137: Same change as line 118.
153-153: Same change as line 118.
169-169: Same change as line 118.
173-173: Allowing ≥ 2 chunks ensures both shell and async content.This assertion confirms that at least two chunks are streamed before completion.
179-179: Error-check for async content is correct.Testing for the exact error message ensures the fallback logic is triggered properly.
200-200: Similar test logic for error messages.
206-224: Tests for streaming a resolved React component confirm correct shell + content flow.This scenario effectively validates the approach for both immediate and eventually returned React elements.
226-240: Covers sync errors in render function thoroughly.Ensures we see the correct fallback path for immediate errors.
242-260: Covers async errors in render function thoroughly.Successfully tests the fallback for promise-based errors.
262-282: Ensures that errors thrown during render function execution are captured.This test confirms the user sees a coherent error message.
284-307: Verifies error emission behavior when throwJsErrors is true.Properly checks that an error event is emitted when the function fails.
309-334: Tests for streaming a plain string from a promise.Great coverage to confirm that plain strings also work in async rendering contexts.
node_package/tests/serverRenderReactComponent.test.ts (5)
1-2: Include this new test file in the TypeScript project configuration.
The lint failure indicates this file is not recognized by the TypeScript compiler. To fix it, please add or reference it in the appropriatetsconfig.jsonso that it's included in the project.Would you like me to generate a script that searches for all references to this file in your repository and queries the
tsconfig.jsonentries to verify inclusion?🧰 Tools
🪛 GitHub Actions: Lint JS and Ruby
[error] 1-1: Parsing error: /home/runner/work/react_on_rails/react_on_rails/node_package/tests/serverRenderReactComponent.test.ts was not found by the project service. Consider either including it in the tsconfig.json or including it in allowDefaultProject.
39-58: Basic rendering test looks good.
This test effectively covers a straightforward scenario of rendering a registered component and verifying the correcthtmlandhasErrorsflags.
108-127: Good negative test for invalid rendered object.
Verifies that returning an object without arenderedHtmlproperty triggers an error, which is the intended behavior.
235-250: Confirm coverage for a promise that returns a function.
Here the promise resolves to a function that itself returns a React element. Consider covering edge cases, such as a promise that resolves after a delay or that rejects mid-render, to ensure resilience of streaming.If you'd like, I can generate an additional test that introduces an artificial delay or rejection path.
252-276: Error-handling test ensures graceful failure.
This test appropriately checks that an invalid scenario triggers a user-facing error message (Exception in rendering!). The coverage for error states seems thorough.node_package/src/streamServerRenderedReactComponent.ts (5)
7-7: Helper import usage is consistent.
TheisServerRenderHashimport is properly utilized below to guard against unsupported server render hashes before streaming.
124-127: Enhanced flexibility for async rendering.
Expanding the parameter toReactElement | Promise<ReactElement | string>correctly accommodates asynchronous or string-based results. Please ensure all upstream calls handle these types without assumptions of synchronicity.
138-152: Centralized error handling approach is good practice.
reportErrorandsendErrorHtmlkeep concerns clearly separated, helping maintain clarity for the streaming logic and potential expansions (e.g., logging).
153-185: Promise resolution logic looks robust.
The code properly handles different result types, logging a console error if a string is returned, then writing that string to the stream. This is consistent with your design choice that raw strings aren’t the intended final behavior but can still be captured.
219-233: Fallback error-handling in the catch block.
Wrapping the streaming logic within atry/catchensures you can handle both runtime and synchronous setup errors gracefully. The final fallback to push error HTML is well-managed.
Judahmeek
left a comment
Judahmeek
left a comment
There was a problem hiding this comment.
Please also figure out the necessary commit steps to get git to recognize that serverRenderReactComponent.test.js has not been deleted, but has been renamed to serverRenderReactComponent.test.ts.
Try renaming first and then making changes in a separate commit.
Having git recognize renamed files makes code reviews a lot easier for those files.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
node_package/tests/serverRenderReactComponent.test.ts (2)
32-36: Consider adding teardown to clean up after tests.While the
beforeEachclears the ComponentRegistry before each test, there's no corresponding cleanup after the tests complete. Consider adding anafterAllto ensure a clean state after the test suite runs.beforeEach(() => { ComponentRegistry.components().clear(); }); + + afterAll(() => { + ComponentRegistry.components().clear(); + });
1-279: Add tests for component promise rejection.While the test suite covers many scenarios, it doesn't test what happens when a Promise is rejected. Consider adding a test case for a render function that returns a rejected Promise to ensure errors are properly handled.
Here's a suggested test implementation:
it('handles rejected promises from render functions', async () => { const errorMessage = 'Async error in render function'; const X9: RenderFunction = (_props: unknown, _railsContext?: RailsContext) => Promise.reject(new Error(errorMessage)); ComponentRegistry.register({ X9 }); const renderResult = serverRenderReactComponent({ name: 'X9', domNodeId: 'myDomId', trace: false, throwJsErrors: false, renderingReturnsPromises: true, }); assertIsPromise(renderResult); const result = await renderResult; expect(result.hasErrors).toBeTruthy(); expect(result.html).toContain('Exception in rendering!'); expect(result.html).toContain(errorMessage); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
eslint.config.ts(1 hunks)node_package/tests/serverRenderReactComponent.test.ts(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Lint JS and Ruby
eslint.config.ts
[error] Parsing error: allowDefaultProject glob 'node_package/tests//*.ts' contains a disallowed ''.
node_package/tests/serverRenderReactComponent.test.ts
[error] Parsing error: allowDefaultProject glob 'node_package/tests//*.ts' contains a disallowed ''.
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: dummy-app-integration-tests (newest)
- GitHub Check: dummy-app-integration-tests (oldest)
🔇 Additional comments (12)
eslint.config.ts (1)
148-148:Details
❓ Verification inconclusive
Fix the glob pattern in allowDefaultProject.
The pipeline is failing because the glob pattern
node_package/tests/**/*.tscontains a disallowed double asterisk ('**'). According to ESLint's configuration, double asterisks are not permitted in theallowDefaultProjectarray.- allowDefaultProject: ['eslint.config.ts', 'knip.ts', 'node_package/tests/**/*.ts'], + allowDefaultProject: ['eslint.config.ts', 'knip.ts', 'node_package/tests/*.ts'],If you need to include TypeScript files in subdirectories, consider specifying them individually or using a different configuration approach that allows recursive patterns.
🏁 Script executed:
#!/bin/bash # Check if there are TypeScript files in subdirectories of node_package/tests find node_package/tests -name "*.ts" | grep -v "^node_package/tests/[^/]*\.ts$" | head -n 5Length of output: 93
Update Glob Pattern in ESLint Configuration
The ESLint rules disallow the use of double asterisks (
**) in theallowDefaultProjectarray. The patch replaces the problematic pattern with a single asterisk to match only files directly undernode_package/tests, which resolves the pipeline failure.Before finalizing this change, please verify whether any TypeScript files are intentionally placed in subdirectories of
node_package/tests. If such files exist, you may need to list them explicitly or adopt a different configuration approach to avoid inadvertently excluding them.- allowDefaultProject: ['eslint.config.ts', 'knip.ts', 'node_package/tests/**/*.ts'], + allowDefaultProject: ['eslint.config.ts', 'knip.ts', 'node_package/tests/*.ts'],node_package/tests/serverRenderReactComponent.test.ts (11)
1-31: Good setup for type safety in tests.The type assertion functions and helper for matching objects provide strong type safety in the tests. This is especially important for testing TypeScript code that handles promises and different return types.
37-57: LGTM: Effective test for basic component rendering.This test case properly verifies the core functionality of rendering a registered React component, with appropriate assertions on the HTML output and error state.
59-82: LGTM: Good error handling test.This test effectively verifies that errors thrown during component rendering are properly caught and reported in the output.
84-106: LGTM: Valid test for renderedHtml property.This test correctly verifies that objects with a
renderedHtmlproperty are processed correctly.
108-127: LGTM: Good test for invalid object returns.This test properly verifies error handling when a render function returns an object without the required
renderedHtmlproperty.
129-161: Well-documented edge case test.The test is preceded by detailed documentation explaining a subtle behavior when rendering functions return strings directly. This is valuable both for test clarity and as documentation for developers.
163-184: LGTM: Good test for async rendering with string HTML.This test correctly verifies the handling of promises that resolve to string HTML, which is a key part of the new async rendering support being added.
186-210: LGTM: Well-documented test for async object returns.The detailed comments explain an important behavior of the component and potential issues. The test effectively validates this behavior.
212-233: LGTM: Good test for react_component_hash helper compatibility.This test demonstrates how async render functions can be used with the
react_component_hashhelper, providing a real-world usage pattern.
235-250: LGTM: Good test for async component rendering.This test correctly verifies that an async function returning a React component works properly, which is a core part of the PR's objective.
252-277: LGTM: Proper error handling test for renderer objects.This test verifies that attempting to render a renderer object (rather than a component) results in an appropriate error.
AbanoubGhadban
force-pushed
the
abanoubghadban/ror1719/add-support-for-async-render-function-returns-component
branch
from
98eed42 to
7c4903d
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
node_package/src/serverRenderReactComponent.ts (1)
30-43: 🛠️ Refactor suggestionImproved handling of
renderedHtmlwith null safety.The changes improve the handling of
renderedHtmlby:
- Removing the explicit type constraint
- Using the nullish coalescing operator (
??) to handle undefined valuesHowever, this handling still doesn't fully address the concern from the previous review about explicitly checking for
nullvalues.Consider implementing more explicit handling:
htmlResult = result.renderedHtml; + + // Line 44 + if (htmlResult === null || htmlResult === undefined) { + htmlResult = ''; + } else if (typeof htmlResult !== 'string') { + htmlResult = JSON.stringify(htmlResult); + }This would provide more predictable behavior and prevent potential issues downstream.
Also applies to: 45-46
🧹 Nitpick comments (10)
docs/javascript/render-functions.md (10)
1-4: Introduction Clarity and Test File Link Reference
The introduction is clear and well laid out. Consider adding a hyperlink for the referenced test file (e.g.[serverRenderReactComponent.test.ts](./serverRenderReactComponent.test.ts)) to improve navigability and help readers quickly locate the source tests.
27-37: Example Section for Objects with Multiple HTML Properties
Examples demonstrate multiple HTML parts well. Consider if a brief note should be added to explain when and why one might return multiple keys (such as for SEO purposes).
73-79: Enhance Consistency in Limitation Descriptions
In the "Important Limitations Observed in Tests" section, consider:
- Consistently capitalizing “HTML” (instead of “html”) for clarity.
- Revising the phrasing for the async functions limitation by inserting an article (e.g. “a Promise‐returning function…”).
- Adding a comma before “and they will still work” to properly separate independent clauses.
These small adjustments will improve the readability of the documentation.🧰 Tools
🪛 LanguageTool
[uncategorized] ~75-~75: You might be missing the article “the” here.
Context: ...nder function causes an error. The testdoesn't render html string returned directly from render functiondemonstrates this. 2. **Non-...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~79-~79: You might be missing the article “a” here.
Context: ...nt Behavior** - Interestingly, the testreturns the object returned by async render function even if it doesn't have renderedHtml propertyshows that Promise-returning ...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~79-~79: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...bjects without arenderedHtmlproperty and they will still work. ## Ruby Helper F...(COMMA_COMPOUND_SENTENCE)
101-101: Remove Trailing Punctuation from Heading
The heading#### When to use:ends with a colon. For consistency with common Markdown style guidelines (e.g. MD026), consider removing the trailing punctuation.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
101-101: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
107-107: Remove Trailing Punctuation from Heading
Similarly, the heading#### Not suitable for:contains a trailing colon. Removing it will align with style guidelines.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
107-107: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
133-133: Remove Trailing Punctuation from Heading
Again, the heading#### When to use:in the react_component_hash section should drop the trailing colon for consistency.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
133-133: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
140-140: Remove Trailing Punctuation from Heading
The heading#### Not suitable for:in this section also has a trailing colon. Please remove it to conform with markdown style guidelines.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
140-140: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
145-145: Remove Trailing Punctuation from Heading
The heading#### Requirements:ends with a colon. For consistency, it is recommended to remove the trailing punctuation.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
145-145: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
223-233: Clarify the Purpose of therenderFunctionFlag in Async Examples
In the asynchronous examples (Return Types 4 & 5), therenderFunctionflag is set to true. Consider adding a brief explanatory note (either inline or as a caption) clarifying that this flag distinguishes asynchronous render functions from synchronous ones. This will help readers understand its significance in the overall render process.
270-292: Return Type 6: Redirect Object – Documentation Clarity
The redirect example is clear and the inline comments effectively communicate the intended behavior. For added clarity, you might consider a short summary sentence after the code block explaining that when a redirect condition is met, the helper renders nothing and instead performs the redirect logic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
docs/javascript/render-functions.md(1 hunks)eslint.config.ts(1 hunks)node_package/src/ReactOnRailsRSC.ts(2 hunks)node_package/src/createReactOutput.ts(2 hunks)node_package/src/handleError.ts(1 hunks)node_package/src/isServerRenderResult.ts(2 hunks)node_package/src/serverRenderReactComponent.ts(6 hunks)node_package/src/serverRenderUtils.ts(1 hunks)node_package/src/streamServerRenderedReactComponent.ts(4 hunks)node_package/src/transformRSCStreamAndReplayConsoleLogs.ts(2 hunks)node_package/src/types/index.ts(4 hunks)node_package/tests/serverRenderReactComponent.test.js(0 hunks)node_package/tests/serverRenderReactComponent.test.ts(1 hunks)node_package/tests/streamServerRenderedReactComponent.test.jsx(6 hunks)
💤 Files with no reviewable changes (1)
- node_package/tests/serverRenderReactComponent.test.js
🚧 Files skipped from review as they are similar to previous changes (10)
- node_package/src/handleError.ts
- eslint.config.ts
- node_package/src/transformRSCStreamAndReplayConsoleLogs.ts
- node_package/src/ReactOnRailsRSC.ts
- node_package/src/serverRenderUtils.ts
- node_package/src/createReactOutput.ts
- node_package/tests/serverRenderReactComponent.test.ts
- node_package/src/isServerRenderResult.ts
- node_package/src/streamServerRenderedReactComponent.ts
- node_package/src/types/index.ts
🧰 Additional context used
🪛 LanguageTool
docs/javascript/render-functions.md
[uncategorized] ~75-~75: You might be missing the article “the” here.
Context: ...nder function causes an error. The test doesn't render html string returned directly from render function demonstrates this. 2. **Non-...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~79-~79: You might be missing the article “a” here.
Context: ...nt Behavior** - Interestingly, the test returns the object returned by async render function even if it doesn't have renderedHtml property shows that Promise-returning ...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~79-~79: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...bjects without a renderedHtml property and they will still work. ## Ruby Helper F...
(COMMA_COMPOUND_SENTENCE)
[style] ~136-~136: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...le HTML strings in an object - When you need to insert rendered content in different pa...
(REP_NEED_TO_VB)
[misspelling] ~147-~147: Did you mean “and”?
Context: ...nts: - The render function MUST return an object - The object MUST include a `com...
(AN_AND)
[uncategorized] ~155-~155: Loose punctuation mark.
Context: ...es errors - either use JSX or wrap in a { renderedHtml: '...' } object. 3. **Async Functions Work wit...
(UNLIKELY_OPENING_PUNCTUATION)
🪛 markdownlint-cli2 (0.17.2)
docs/javascript/render-functions.md
101-101: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
107-107: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
133-133: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
140-140: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
145-145: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: dummy-app-integration-tests (oldest)
- GitHub Check: dummy-app-integration-tests (newest)
- GitHub Check: rspec-package-tests (oldest)
- GitHub Check: build
🔇 Additional comments (18)
node_package/tests/streamServerRenderedReactComponent.test.jsx (7)
63-97: Well-structured enhancement to support multiple component types.The addition of the
componentTypeparameter and the switch statement is a good approach to test different component rendering scenarios. This implementation covers all necessary test cases for the new async render function support.
118-118: Good improvement: changedontooncefor 'end' event listeners.Changing from
ontooncefor end event listeners prevents potential memory leaks and ensures the callback is executed exactly once when the stream ends.Also applies to: 137-137, 153-153, 169-169, 190-190
173-173: Appropriate test assertion update.Changing from
toHaveLengthtotoBeGreaterThanOrEqualprovides more flexibility in the tests, acknowledging that the exact number of chunks might vary but should meet a minimum threshold.Also applies to: 194-194
206-224: Good test coverage for sync and async render functions.Using parameterized tests with
it.eachis an efficient way to test bothasyncRenderFunctionandrenderFunctionwith the same expectations.
226-260: Comprehensive error handling tests.These test cases properly verify that both synchronous and asynchronous errors are handled correctly in render functions.
262-307: Good coverage for erroneous render functions.The tests correctly verify handling of errors thrown directly from render functions and the proper emission of those errors when
throwJsErrorsis true.
309-334: Important test case for string promise resolution.This test is crucial as it verifies that the system can properly handle promises that resolve to strings, not just React elements, which expands the flexibility of the API.
node_package/src/serverRenderReactComponent.ts (6)
1-3: Added React import for element validation.Good addition of the React import which enables the
isValidElementcheck used later in the code. This makes the dependencies more explicit.
18-19: New type imports support async React element handling.These new types support the async rendering functionality and are crucial for the type safety of the updated code.
48-57: NewprocessReactElementfunction with helpful error messages.This is a good addition that encapsulates the rendering of React elements with proper error handling. The detailed error message will help developers understand and fix issues related to render functions vs. React Function Components.
59-62: Updated signature forprocessPromisefunction.The function now accepts
CreateReactOutputAsyncResultinstead ofPromise<string>and returnsRenderStateHtml. This provides better type safety and flexibility.
71-76: Enhanced promise handling to support React elements.The updated promise handling now checks if the resolved value is a valid React element and processes it accordingly. This is a key enhancement for supporting async render functions that return React elements.
However, this change might require updates to ReactOnRails Pro's NodeRenderer as noted in a previous review comment.
Please verify compatibility with ReactOnRails Pro's NodeRenderer, as this change in behavior might affect it.
85-87:Details
❓ Verification inconclusive
Handling of non-promise, non-server-render-hash results has changed.
Line 86 now directly calls
processReactElementfor non-promise, non-server-render-hash results, implying these results are expected to be React elements rather than strings. This is a significant behavioral change.Please verify that all existing code that uses this function now returns React elements instead of strings, or update the code to handle both cases.
🏁 Script executed:
#!/bin/bash # Search for places where components or render functions might return strings directly rg -A 3 "ComponentRegistry.register|registerComponent" --type=js --type=jsx --type=ts --type=tsxLength of output: 126
Action: Confirm React Element Consistency in
serverRenderReactComponentUsageThe updated function now processes non-promise, non-server-render-hash results via
processReactElement, which means callers are now expected to return React elements instead of strings. However, the initial verification command failed due to the unrecognized file type "jsx". To ensure that no callers still return strings, please run the following updated verification script that uses glob patterns to include JavaScript, JSX, TypeScript, and TSX files:#!/bin/bash # Search for occurrences of ComponentRegistry.register or registerComponent in various file types rg -g "*.js" -g "*.jsx" -g "*.ts" -g "*.tsx" -A 3 "ComponentRegistry\.register|registerComponent"After executing this command manually, confirm that all usage of the function now returns React elements. If any cases still return strings, update those call sites accordingly.
docs/javascript/render-functions.md (5)
7-10: Example Section for React Components (JSX)
The first example clearly demonstrates a simple JSX return. The code snippet is clear and concise. No changes needed!
17-25: Example Section for Objects withrenderedHtmlProperty
The code example effectively illustrates returning an object with arenderedHtmlproperty. The formatting and clarity are good.
39-46: Example Section for Promises of Strings
The async example is straightforward. The use ofasyncandawaitis correct and clear.
48-57: Example Section for Promises of Objects
This snippet correctly shows how to return an object from an async function. The example is clear.
59-69: Example Section for Redirect Information
The redirect example is precise and the inline comments clearly explain the purpose of each key. No modifications are necessary here.
AbanoubGhadban
force-pushed
the
abanoubghadban/ror1719/add-support-for-async-render-function-returns-component
branch
from
7c4903d to
77d7cea
Compare
AbanoubGhadban
changed the base branch from
master
to
rename-server-render-react-component-test-to-ts
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/javascript/render-functions.md (1)
27-27: Section title could be more descriptiveThe section title "Objects with Multiple HTML Properties" should better describe that these objects must include a
componentHtmlproperty.Consider renaming to "Objects with componentHtml Property (and Additional HTML Properties)" to match previous review feedback and better describe the content requirements.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
docs/javascript/render-functions.md(1 hunks)eslint.config.ts(1 hunks)node_package/src/ReactOnRailsRSC.ts(2 hunks)node_package/src/createReactOutput.ts(2 hunks)node_package/src/handleError.ts(1 hunks)node_package/src/isServerRenderResult.ts(2 hunks)node_package/src/serverRenderReactComponent.ts(6 hunks)node_package/src/serverRenderUtils.ts(1 hunks)node_package/src/streamServerRenderedReactComponent.ts(4 hunks)node_package/src/transformRSCStreamAndReplayConsoleLogs.ts(2 hunks)node_package/src/types/index.ts(4 hunks)node_package/tests/serverRenderReactComponent.test.js(0 hunks)node_package/tests/serverRenderReactComponent.test.ts(1 hunks)node_package/tests/streamServerRenderedReactComponent.test.jsx(6 hunks)
💤 Files with no reviewable changes (1)
- node_package/tests/serverRenderReactComponent.test.js
🚧 Files skipped from review as they are similar to previous changes (9)
- node_package/src/transformRSCStreamAndReplayConsoleLogs.ts
- node_package/src/handleError.ts
- eslint.config.ts
- node_package/src/isServerRenderResult.ts
- node_package/src/serverRenderReactComponent.ts
- node_package/tests/serverRenderReactComponent.test.ts
- node_package/src/createReactOutput.ts
- node_package/src/streamServerRenderedReactComponent.ts
- node_package/src/serverRenderUtils.ts
🧰 Additional context used
🧬 Code Definitions (2)
node_package/src/ReactOnRailsRSC.ts (2)
node_package/src/types/index.ts (1)
RSCRenderParams(156-158)node_package/src/loadReactClientManifest.ts (1)
loadReactClientManifest(7-24)
node_package/tests/streamServerRenderedReactComponent.test.jsx (4)
lib/react_on_rails/react_component/render_options.rb (1)
props(26-28)spec/dummy/client/app/store/composeInitialState.js (1)
props(3-3)spec/dummy/client/app/startup/ReduxSharedStoreApp.client.jsx (1)
props(16-50)node_package/src/ReactOnRails.client.ts (1)
streamServerRenderedReactComponent(165-169)
🪛 LanguageTool
docs/javascript/render-functions.md
[uncategorized] ~75-~75: You might be missing the article “the” here.
Context: ...nder function causes an error. The test doesn't render html string returned directly from render function demonstrates this. 2. **Non-...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~79-~79: You might be missing the article “a” here.
Context: ...nt Behavior** - Interestingly, the test returns the object returned by async render function even if it doesn't have renderedHtml property shows that Promise-returning ...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~79-~79: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...bjects without a renderedHtml property and they will still work. ## Ruby Helper F...
(COMMA_COMPOUND_SENTENCE)
[style] ~136-~136: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...le HTML strings in an object - When you need to insert rendered content in different pa...
(REP_NEED_TO_VB)
[misspelling] ~147-~147: Did you mean “and”?
Context: ...nts: - The render function MUST return an object - The object MUST include a `com...
(AN_AND)
[uncategorized] ~155-~155: Loose punctuation mark.
Context: ...es errors - either use JSX or wrap in a { renderedHtml: '...' } object. 3. **Async Functions Work wit...
(UNLIKELY_OPENING_PUNCTUATION)
🪛 markdownlint-cli2 (0.17.2)
docs/javascript/render-functions.md
101-101: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
107-107: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
133-133: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
140-140: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
145-145: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: dummy-app-integration-tests (newest)
- GitHub Check: dummy-app-integration-tests (oldest)
🔇 Additional comments (28)
node_package/src/ReactOnRailsRSC.ts (2)
24-26: Support added for async render function returnsThe function signature has been enhanced to accept a promise that resolves to either a React element or a string, which aligns with the PR objective of supporting async render functions.
37-38: Good use of Promise.all for parallel executionUsing
Promise.allallows concurrent resolution of both the React client manifest and the render function result, which is more efficient than sequential execution.docs/javascript/render-functions.md (7)
3-3: Good addition of test file referenceIncluding the reference to the test file provides helpful context for understanding the documentation.
79-79: Revise the wording about async function behaviorThe word "Interestingly" doesn't add value and the explanation could be clearer about why async functions don't require the
renderedHtmlproperty.Consider rewording to: "Async Functions Have Different Behavior - Promise-returning functions can return objects without a
renderedHtmlproperty, and they will still work because async functions are processed differently in the rendering pipeline."🧰 Tools
🪛 LanguageTool
[uncategorized] ~79-~79: You might be missing the article “a” here.
Context: ...nt Behavior** - Interestingly, the testreturns the object returned by async render function even if it doesn't have renderedHtml propertyshows that Promise-returning ...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[uncategorized] ~79-~79: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...bjects without arenderedHtmlproperty and they will still work. ## Ruby Helper F...(COMMA_COMPOUND_SENTENCE)
99-99: Consider consolidating redundant sectionsThe examples of
react_componentwith different return types all look the same in this section, which doesn't effectively illustrate the differences.Replace this section with a single statement explaining that
react_componentaccepts render functions that return JSX, objects with arenderedHtmlproperty, and promises, then refer to the more detailed examples in the section below.
110-110: Add examples of content types for different page partsBe more specific about what kind of content can be inserted in different parts of the page.
Add examples such as "meta tags, title tags, and style tags" to provide clearer guidance on what types of content can be distributed across the page.
114-114: Add return type acceptance informationThis section should specify which render function return types it accepts, similar to the
react_componentsection.Add a statement clarifying which return types are accepted by
react_component_hash, such as "This helper accepts render functions that return objects with acomponentHtmlproperty and additional HTML properties."
151-151: Remove reference to tests as authorityPhrases like "Based on the Tests" and "tests show" give the impression that tests are the authority rather than the implementation.
Revise the heading to "Best Practices" and remove references to tests as the source of authority.
157-157: Redundant information with earlier sectionsThis section repeats information already covered in lines 73-79.
Consider removing or consolidating this redundant information to improve document flow and clarity.
node_package/tests/streamServerRenderedReactComponent.test.jsx (10)
63-64: Good addition of componentType parameterAdding the
componentTypeparameter tosetupStreamTestallows for testing different component rendering scenarios, which is crucial for verifying the new async render function support.
65-97: Well-structured component type handlingThe switch statement provides a clean way to register different types of components for testing, covering both synchronous and asynchronous scenarios, as well as error cases.
118-118: Better event handling with onceChanging from
on('end', resolve)toonce('end', resolve)prevents potential memory leaks by ensuring the callback is only called once.
173-173: More flexible test assertionsUsing
toBeGreaterThanOrEqualinstead oftoHaveLengthprovides more flexibility when testing asynchronous operations that may produce a variable number of chunks.
206-224: Comprehensive test coverage for different component typesThis parametrized test effectively verifies that both synchronous and asynchronous render functions can produce the same output when resolved properly.
226-240: Good error handling testing for sync errorsThis test ensures that synchronous errors are properly caught and handled in both types of render functions.
242-260: Good error handling testing for async errorsThis test ensures that asynchronous errors are properly caught and handled in both types of render functions.
262-282: Comprehensive testing of erroneous component scenariosThis test ensures that errors occurring within the render functions themselves (rather than in the rendered components) are properly caught and handled.
284-307: Complete error propagation testingThis test verifies that errors are properly propagated to the error handler when throwJsErrors is true, which is critical for proper error reporting.
309-334: Test for string promise resolutionThis test verifies the ability to handle promises that resolve directly to strings, which is important for the new async render function support.
node_package/src/types/index.ts (9)
48-51: Good addition of ServerRenderHashRenderedHtml typeThis type provides a clear structure for objects that contain rendered HTML, requiring a
componentHtmlproperty and allowing additional string properties.
54-54: Improved type safety for renderedHtml propertyUsing the new
ServerRenderHashRenderedHtmltype for therenderedHtmlproperty provides better type safety and documentation.
60-64: Enhanced type structure for CreateReactOutputResultBreaking down the result types into synchronous and asynchronous variants improves type clarity and allows for more precise type checking.
66-70: Improved type structure for RenderFunctionResultSimilar to the CreateReactOutputResult changes, this breakdown into synchronous and asynchronous variants provides better type safety and documentation.
176-176: Addition of FinalHtmlResult typeThis union type clearly defines what can be used as the final HTML result, improving type safety throughout the codebase.
179-179: Updated RenderResult to use FinalHtmlResultThis change ensures consistency in the types used for HTML results and improves type safety.
186-188: Addition of RSCPayloadChunk interfaceThis interface specifically constrains the html property to be a string, which is important for RSC payload chunks that must be stringified.
378-378: Addition of RenderStateHtml typeThis type provides a clear definition for what can be used as the HTML in the render state, including both synchronous and asynchronous results.
381-381: Updated RenderState to use RenderStateHtmlThis change ensures consistency in the types used for HTML results in the render state and improves type safety.
I renamed the file at the #1723 , it should be merged before merging this file |
There was a problem hiding this comment.
Pull Request Overview
This PR aims to add support for async render function returns by updating various components in the test suite, server rendering logic, and type definitions, while also enhancing documentation.
- Updated tests to support additional component types and async error handling.
- Extended TypeScript types and modified streaming and server rendering functions to deal with Promise-based render function outputs.
- Added comprehensive documentation for render functions and their supported return types.
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| node_package/tests/streamServerRenderedReactComponent.test.jsx | Updates tests to use .once for stream events, add support for new component types, and revise expected error output formatting. |
| node_package/tests/serverRenderReactComponent.test.ts | Refactors tests to use strict type assertions and updated render parameters for improved clarity with async and sync render functions. |
| node_package/src/types/index.ts | Introduces new types to better support async render functions and server render hashes. |
| node_package/src/streamServerRenderedReactComponent.ts | Updates streaming logic to handle Promise-based React components or string return values with improved error reporting. |
| node_package/src/serverRenderUtils.ts, serverRenderReactComponent.ts, isServerRenderResult.ts, handleError.ts, createReactOutput.ts, ReactOnRailsRSC.ts | Refactors error handling, conversion functions and type processing to support the revised return types from render functions. |
| eslint.config.ts | Updates allowed default projects to include additional test files. |
| docs/javascript/render-functions.md | Adds a detailed usage guide on render functions, their return types, and proper usage with Rails helpers. |
Comments suppressed due to low confidence (1)
node_package/src/createReactOutput.ts:6
- [nitpick] The warning message logged when a render function returns a React Element directly is quite verbose; consider clarifying or simplifying the message and updating the documentation to clearly state that render functions should return a React component function rather than a JSX element.
function createReactElementFromRenderFunctionResult(renderFunctionResult: ReactComponent, name: string, props: Record<string, unknown> | undefined): React.ReactElement {
|
|
||
| 2. **Objects Require Specific Properties** - Non-promise objects must include a `renderedHtml` property to be valid when used with `react_component`. | ||
|
|
||
| 3. **Async Functions Support All Return Types** - Async functions can return React components, strings, or objects with any property structure due to special handling in the server renderer, but it doesn't support properties like redirectLocation and routingError that can be returned by sync render function. |
There was a problem hiding this comment.
Probably redirectLocation and routingError should be described above (under "7. Redirect Information"?) and this should say something like
but it doesn't support redirect information [or "routing information"? and it should have a link to that] that can be returned by sync render function.
There was a problem hiding this comment.
I already mentioned at the 7. Redirect Information section that
React on Rails will not handle the actual redirection to another page. It will just return an empty component and depend on the front end to handle the redirection when it renders the router on the front end.
I updated this content to have a link to the above section
There was a problem hiding this comment.
Personally, I don't believe the redirectLocation property serves any purpose. It merely logs the location to which a redirection should occur, doesn’t it, @Judahmeek?
There was a problem hiding this comment.
I believe that's right. Looks like a redirectLocation return is just to cut out any unnecessary processing.
AbanoubGhadban
changed the title
AbanoubGhadban
changed the title
Judahmeek
left a comment
Judahmeek
left a comment
There was a problem hiding this comment.
Should this PR also be updating spec/dummy to use the async render functions?
AbanoubGhadban
force-pushed
the
abanoubghadban/ror1719/add-support-for-async-render-function-returns-component
branch
from
0306315 to
e467df4
Compare
AbanoubGhadban
force-pushed
the
rename-server-render-react-component-test-to-ts
branch
from
ed16293 to
4b341f9
Compare
@Judahmeek We can only use promises in the node renderer, so I added an example to the dummy app of RORP. Please check it here |
AbanoubGhadban
deleted the
abanoubghadban/ror1719/add-support-for-async-render-function-returns-component
branch
Enhanced Support for Async Render Functions Returning React Components
Summary
This PR enhances React on Rails to support async render functions that return React components. This completes the support for all major render function return types in both synchronous and asynchronous contexts.
Feature Details
Async render functions can now return React components directly, enabling a cleaner pattern for components that need to fetch data before rendering:
Then, render it in your Rails view:
Architectural Advantages
Instead of requiring developers to handle string rendering themselves (by calling
renderToStringinside their async functions), this approach provides significant flexibility and future-proofing:react_componenthelper function or stream it by callingstream_react_componentwithout creating different implementations for each componentRelated Documentation
This feature completes the "Return Type 6: Promises of React Components" pattern described in our documentation.
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
New Features
Bug Fixes