From 71bba147e5c87671ba8ae86ec78e5e332019f825 Mon Sep 17 00:00:00 2001 From: Abanoub Ghadban Date: Fri, 11 Oct 2024 14:21:24 +0300 Subject: [PATCH 1/9] refactor serverRenderReactComponent --- node_package/src/isServerRenderResult.ts | 6 +- .../src/serverRenderReactComponent.ts | 265 +++++++++--------- node_package/src/types/index.ts | 8 + 3 files changed, 151 insertions(+), 128 deletions(-) diff --git a/node_package/src/isServerRenderResult.ts b/node_package/src/isServerRenderResult.ts index 3b7db625a..88f9cf5be 100644 --- a/node_package/src/isServerRenderResult.ts +++ b/node_package/src/isServerRenderResult.ts @@ -9,7 +9,7 @@ export function isServerRenderHash(testValue: CreateReactOutputResult): (testValue as ServerRenderResult).error); } -export function isPromise(testValue: CreateReactOutputResult): - testValue is Promise { - return !!((testValue as Promise).then); +export function isPromise(testValue: CreateReactOutputResult | Promise | string | null): + testValue is Promise { + return !!((testValue as Promise | null)?.then); } diff --git a/node_package/src/serverRenderReactComponent.ts b/node_package/src/serverRenderReactComponent.ts index 5a518e3bf..986870de1 100644 --- a/node_package/src/serverRenderReactComponent.ts +++ b/node_package/src/serverRenderReactComponent.ts @@ -6,154 +6,168 @@ import createReactOutput from './createReactOutput'; import { isPromise, isServerRenderHash } from './isServerRenderResult'; import buildConsoleReplay from './buildConsoleReplay'; import handleError from './handleError'; -import type { RenderParams, RenderResult, RenderingError, ServerRenderResult } from './types'; +import type { CreateReactOutputResult, RegisteredComponent, RenderParams, RenderResult, RenderingError, ServerRenderResult } from './types'; -/* eslint-disable @typescript-eslint/no-explicit-any */ +type RenderState = { + result: null | string | Promise; + hasErrors: boolean; + error: null | RenderingError; +}; -function serverRenderReactComponentInternal(options: RenderParams): null | string | Promise { - const { name, domNodeId, trace, props, railsContext, renderingReturnsPromises, throwJsErrors } = options; +type RenderOptions = { + name: string; + domNodeId?: string; + trace?: boolean; + renderingReturnsPromises: boolean; +}; + +function validateComponent(componentObj: RegisteredComponent, name: string) { + if (componentObj.isRenderer) { + throw new Error(`Detected a renderer while server rendering component '${name}'. See https://github.com/shakacode/react_on_rails#renderer-functions`); + } +} - let renderResult: null | string | Promise = null; - let hasErrors = false; - let renderingError: null | RenderingError = null; +function processServerRenderHash(result: ServerRenderResult, options: RenderOptions): string { + const { redirectLocation, routeError } = result; + const hasErrors = !!routeError; - try { - const componentObj = ComponentRegistry.get(name); - if (componentObj.isRenderer) { - throw new Error(`\ -Detected a renderer while server rendering component '${name}'. \ -See https://github.com/shakacode/react_on_rails#renderer-functions`); + if (hasErrors) { + console.error(`React Router ERROR: ${JSON.stringify(routeError)}`); + } + + if (redirectLocation) { + if (options.trace) { + const redirectPath = redirectLocation.pathname + redirectLocation.search; + console.log(`ROUTER REDIRECT: ${options.name} to dom node with id: ${options.domNodeId}, redirect to ${redirectPath}`); } + return ''; + } - const reactRenderingResult = createReactOutput({ - componentObj, - domNodeId, - trace, - props, - railsContext, - }); - - const processServerRenderHash = () => { - // We let the client side handle any redirect - // Set hasErrors in case we want to throw a Rails exception - const { redirectLocation, routeError } = reactRenderingResult as ServerRenderResult; - hasErrors = !!routeError; - - if (hasErrors) { - console.error( - `React Router ERROR: ${JSON.stringify(routeError)}`, - ); - } - - if (redirectLocation) { - if (trace) { - const redirectPath = redirectLocation.pathname + redirectLocation.search; - console.log(`\ - ROUTER REDIRECT: ${name} to dom node with id: ${domNodeId}, redirect to ${redirectPath}`, - ); - } - // For redirects on server rendering, we can't stop Rails from returning the same result. - // Possibly, someday, we could have the rails server redirect. - return ''; - } - return (reactRenderingResult as ServerRenderResult).renderedHtml as string; - }; + return result.renderedHtml as string; +} - const processPromise = () => { - if (!renderingReturnsPromises) { - console.error('Your render function returned a Promise, which is only supported by a node renderer, not ExecJS.'); - } - return reactRenderingResult; - }; +function processPromise(result: Promise, renderingReturnsPromises: boolean): Promise | string { + if (!renderingReturnsPromises) { + console.error('Your render function returned a Promise, which is only supported by a node renderer, not ExecJS.'); + // If the app is using server rendering with ExecJS, then the promise will not be awaited. + // And when a promise is passed to JSON.stringify, it will be converted to '{}'. + return '{}'; + } + return result as Promise; +} - const processReactElement = () => { - try { - return ReactDOMServer.renderToString(reactRenderingResult as ReactElement); - } catch (error) { - console.error(`Invalid call to renderToString. Possibly you have a renderFunction, a function that already +function processReactElement(result: ReactElement): string { + try { + return ReactDOMServer.renderToString(result); + } 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.`); - throw error; - } - }; - - if (isServerRenderHash(reactRenderingResult)) { - renderResult = processServerRenderHash(); - } else if (isPromise(reactRenderingResult)) { - renderResult = processPromise() as Promise; - } else { - renderResult = processReactElement(); - } - } catch (e: any) { - if (throwJsErrors) { - throw e; - } + throw error; + } +} - hasErrors = true; - renderResult = handleError({ - e, - name, - serverSide: true, - }); - renderingError = e; +function processRenderingResult(result: CreateReactOutputResult, options: RenderOptions): string | Promise { + if (isServerRenderHash(result)) { + return processServerRenderHash(result, options); } + if (isPromise(result)) { + return processPromise(result, options.renderingReturnsPromises); + } + return processReactElement(result); +} - const consoleReplayScript = buildConsoleReplay(); - const addRenderingErrors = (resultObject: RenderResult, renderError: RenderingError) => { - // Do not use `resultObject.renderingError = renderError` because JSON.stringify will turn it into '{}'. - resultObject.renderingError = { // eslint-disable-line no-param-reassign - message: renderError.message, - stack: renderError.stack, - }; +function handleRenderingError(e: Error, renderState: RenderState, options: { name: string, throwJsErrors: boolean }) { + if (options.throwJsErrors) { + throw e; + } + return { + ...renderState, + hasErrors: true, + result: handleError({ e, name: options.name, serverSide: true }), + error: e, }; +} - if (renderingReturnsPromises) { - const resolveRenderResult = async () => { - let promiseResult: RenderResult; - - try { - promiseResult = { - html: await renderResult, - consoleReplayScript, - hasErrors, - }; - } catch (e: any) { - if (throwJsErrors) { - throw e; - } - promiseResult = { - html: handleError({ - e, - name, - serverSide: true, - }), - consoleReplayScript, - hasErrors: true, - }; - renderingError = e; - } - - if (renderingError !== null) { - addRenderingErrors(promiseResult, renderingError); - } - - return promiseResult; +function createResultObject(html: string | null, consoleReplayScript: string, hasErrors: boolean, error: RenderingError | null): RenderResult { + const result: RenderResult = { html, consoleReplayScript, hasErrors }; + if (error) { + result.renderingError = { + message: error.message, + stack: error.stack, }; + } + return result; +} + +function createSyncResult(renderState: RenderState & { result: string | null }, consoleReplayScript: string): RenderResult { + return createResultObject(renderState.result, consoleReplayScript, renderState.hasErrors, renderState.error); +} - return resolveRenderResult(); +function createPromiseResult(renderState: RenderState & { result: Promise }, consoleReplayScript: string): Promise { + return (async () => { + try { + const html = await renderState.result; + return createResultObject(html, consoleReplayScript, renderState.hasErrors, renderState.error); + } catch (e: unknown) { + const error = e instanceof Error ? e : new Error(String(e)); + const html = handleError({ e: error, name: 'Unknown', serverSide: true }); + return createResultObject(html, consoleReplayScript, true, error); + } + })(); +} + +function createFinalResult(renderState: RenderState): null | string | Promise { + const consoleReplayScript = buildConsoleReplay(); + + const { result } = renderState; + if (isPromise(result)) { + return createPromiseResult({ ...renderState, result }, consoleReplayScript); } - const result: RenderResult = { - html: renderResult as string, - consoleReplayScript, - hasErrors, + return JSON.stringify(createSyncResult({ ...renderState, result }, consoleReplayScript)); +} + +function serverRenderReactComponentInternal(options: RenderParams): null | string | Promise { + const { name, domNodeId, trace, props, railsContext, renderingReturnsPromises, throwJsErrors } = options; + + let renderState: RenderState = { + result: null, + hasErrors: false, + error: null, }; - if (renderingError) { - addRenderingErrors(result, renderingError); + try { + const componentObj = ComponentRegistry.get(name); + validateComponent(componentObj, name); + + // Renders the component or executes the render function + // - If the registered component is a React element or component, it renders it + // - If it's a render function, it executes the function and processes the result: + // - For React elements or components, it renders them + // - For promises, it returns them without awaiting (for async rendering) + // - For other values (e.g., strings), it returns them directly + // Note: Only synchronous operations are performed at this stage + const reactRenderingResult = createReactOutput({ componentObj, domNodeId, trace, props, railsContext }); + // Processes the result from createReactOutput: + // 1. Converts React elements to HTML strings + // 2. Returns rendered HTML from serverRenderHash + // 3. Handles promises for async rendering + renderState.result = processRenderingResult(reactRenderingResult, { name, domNodeId, trace, renderingReturnsPromises }); + } catch (e) { + renderState = handleRenderingError(e as Error, renderState, { name, throwJsErrors }); } - return JSON.stringify(result); + // Finalize the rendering result and prepare it for server response + // 1. Builds the consoleReplayScript for client-side console replay + // 2. Handles both synchronous and asynchronous (Promise) results + // 3. Constructs a JSON object with the following properties: + // - html: string | null (The rendered component HTML) + // - consoleReplayScript: string (Script to replay console outputs on the client) + // - hasErrors: boolean (Indicates if any errors occurred during rendering) + // - renderingError: Error | null (The error object if an error occurred, null otherwise) + // 4. For Promise results, it awaits resolution before creating the final JSON + return createFinalResult(renderState); } const serverRenderReactComponent: typeof serverRenderReactComponentInternal = (options) => { @@ -165,4 +179,5 @@ const serverRenderReactComponent: typeof serverRenderReactComponentInternal = (o console.history = []; } }; + export default serverRenderReactComponent; diff --git a/node_package/src/types/index.ts b/node_package/src/types/index.ts index a952b4b15..70bc03522 100644 --- a/node_package/src/types/index.ts +++ b/node_package/src/types/index.ts @@ -67,7 +67,15 @@ export type { // eslint-disable-line import/prefer-default-export export interface RegisteredComponent { name: string; component: ReactComponentOrRenderFunction; + // Indicates if the registered component is a Render-function. + // Render-functions receive two arguments: props and railsContext. + // They should return a string, React component, React element, or a Promise. + // These functions are used to create dynamic React components or server-rendered HTML. renderFunction: boolean; + // Indicates if the registered component is a Renderer function. + // Renderer function handles DOM rendering or hydration with 3 args: (props, railsContext, domNodeId) + // Supported on the client side only. + // All renderer functions are render functions, but not all render functions are renderer functions. isRenderer: boolean; } From 448cd5dd62a9a0cdc882a87da43a933891c98b22 Mon Sep 17 00:00:00 2001 From: Abanoub Ghadban Date: Fri, 11 Oct 2024 14:21:46 +0300 Subject: [PATCH 2/9] add stream package to the dummy app to fix the build problem --- spec/dummy/package.json | 1 + spec/dummy/yarn.lock | 14 +++++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/spec/dummy/package.json b/spec/dummy/package.json index 44a437380..f7b5c11d7 100644 --- a/spec/dummy/package.json +++ b/spec/dummy/package.json @@ -52,6 +52,7 @@ "sass-loader": "^12.3.0", "sass-resources-loader": "^2.1.0", "shakapacker": "8.0.0", + "stream": "^0.0.3", "style-loader": "^3.3.1", "terser-webpack-plugin": "5.3.1", "url-loader": "^4.0.0", diff --git a/spec/dummy/yarn.lock b/spec/dummy/yarn.lock index 1a634f47d..93bc35abc 100644 --- a/spec/dummy/yarn.lock +++ b/spec/dummy/yarn.lock @@ -3375,6 +3375,11 @@ component-emitter@^1.2.1: resolved "https://registry.yarnpkg.com/component-emitter/-/component-emitter-1.3.0.tgz#16e4070fba8ae29b679f2215853ee181ab2eabc0" integrity sha512-Rd3se6QB+sO1TwqZjscQrurpEPIfO0/yYnSin6Q/rD3mOutHvUrCAhJub3r90uNb+SESBuE0QYoB90YdfatsRg== +component-emitter@^2.0.0: + version "2.0.0" + resolved "https://registry.yarnpkg.com/component-emitter/-/component-emitter-2.0.0.tgz#3a137dfe66fcf2efe3eab7cb7d5f51741b3620c6" + integrity sha512-4m5s3Me2xxlVKG9PkZpQqHQR7bgpnN7joDMJ4yvVkVXngjoITG76IaZmzmywSeRTeTpc6N6r3H3+KyUurV8OYw== + compose-function@3.0.3: version "3.0.3" resolved "https://registry.yarnpkg.com/compose-function/-/compose-function-3.0.3.tgz#9ed675f13cc54501d30950a486ff6a7ba3ab185f" @@ -6452,7 +6457,7 @@ react-is@^18.0.0: integrity sha512-xWGDIW6x921xtzPkhiULtthJHoJvBbF3q26fzloPCK0hsvxtPVelvftw3zjbHWSkR2km9Z+4uxbDDK/6Zw9B8w== "react-on-rails@file:.yalc/react-on-rails": - version "14.0.0" + version "14.0.4" dependencies: "@babel/runtime-corejs3" "^7.12.5" @@ -7295,6 +7300,13 @@ stream-http@^2.7.2: to-arraybuffer "^1.0.0" xtend "^4.0.0" +stream@^0.0.3: + version "0.0.3" + resolved "https://registry.yarnpkg.com/stream/-/stream-0.0.3.tgz#3f3934a900a561ce3e2b9ffbd2819cead32699d9" + integrity sha512-aMsbn7VKrl4A2T7QAQQbzgN7NVc70vgF5INQrBXqn4dCXN1zy3L9HGgLO5s7PExmdrzTJ8uR/27aviW8or8/+A== + dependencies: + component-emitter "^2.0.0" + string-width@^3.0.0, string-width@^3.1.0: version "3.1.0" resolved "https://registry.yarnpkg.com/string-width/-/string-width-3.1.0.tgz#22767be21b62af1081574306f69ac51b62203961" From fa48d6f38320310529a943940e6fd08cb0a8c6f0 Mon Sep 17 00:00:00 2001 From: Abanoub Ghadban Date: Sun, 13 Oct 2024 15:12:56 +0300 Subject: [PATCH 3/9] improve naming and organization --- .../src/serverRenderReactComponent.ts | 103 +++++++++--------- 1 file changed, 53 insertions(+), 50 deletions(-) diff --git a/node_package/src/serverRenderReactComponent.ts b/node_package/src/serverRenderReactComponent.ts index 986870de1..8f6f365bf 100644 --- a/node_package/src/serverRenderReactComponent.ts +++ b/node_package/src/serverRenderReactComponent.ts @@ -11,23 +11,23 @@ import type { CreateReactOutputResult, RegisteredComponent, RenderParams, Render type RenderState = { result: null | string | Promise; hasErrors: boolean; - error: null | RenderingError; + error?: RenderingError; }; type RenderOptions = { - name: string; + componentName: string; domNodeId?: string; trace?: boolean; renderingReturnsPromises: boolean; }; -function validateComponent(componentObj: RegisteredComponent, name: string) { +function validateComponent(componentObj: RegisteredComponent, componentName: string) { if (componentObj.isRenderer) { - throw new Error(`Detected a renderer while server rendering component '${name}'. See https://github.com/shakacode/react_on_rails#renderer-functions`); + throw new Error(`Detected a renderer while server rendering component '${componentName}'. See https://github.com/shakacode/react_on_rails#renderer-functions`); } } -function processServerRenderHash(result: ServerRenderResult, options: RenderOptions): string { +function processServerRenderHash(result: ServerRenderResult, options: RenderOptions): RenderState { const { redirectLocation, routeError } = result; const hasErrors = !!routeError; @@ -35,15 +35,18 @@ function processServerRenderHash(result: ServerRenderResult, options: RenderOpti console.error(`React Router ERROR: ${JSON.stringify(routeError)}`); } + let htmlResult: string; if (redirectLocation) { if (options.trace) { const redirectPath = redirectLocation.pathname + redirectLocation.search; - console.log(`ROUTER REDIRECT: ${options.name} to dom node with id: ${options.domNodeId}, redirect to ${redirectPath}`); + console.log(`ROUTER REDIRECT: ${options.componentName} to dom node with id: ${options.domNodeId}, redirect to ${redirectPath}`); } - return ''; + htmlResult = ''; + } else { + htmlResult = result.renderedHtml as string; } - return result.renderedHtml as string; + return { result: htmlResult, hasErrors }; } function processPromise(result: Promise, renderingReturnsPromises: boolean): Promise | string { @@ -67,79 +70,78 @@ as a renderFunction and not a simple React Function Component.`); } } -function processRenderingResult(result: CreateReactOutputResult, options: RenderOptions): string | Promise { +function processRenderingResult(result: CreateReactOutputResult, options: RenderOptions): RenderState { if (isServerRenderHash(result)) { return processServerRenderHash(result, options); } if (isPromise(result)) { - return processPromise(result, options.renderingReturnsPromises); + return { result: processPromise(result, options.renderingReturnsPromises), hasErrors: false }; } - return processReactElement(result); + return { result: processReactElement(result), hasErrors: false }; } -function handleRenderingError(e: Error, renderState: RenderState, options: { name: string, throwJsErrors: boolean }) { +function handleRenderingError(e: unknown, options: { componentName: string, throwJsErrors: boolean }) { if (options.throwJsErrors) { throw e; } + const error = e instanceof Error ? e : new Error(String(e)); return { - ...renderState, hasErrors: true, - result: handleError({ e, name: options.name, serverSide: true }), - error: e, + result: handleError({ e: error, name: options.componentName, serverSide: true }), + error, }; } -function createResultObject(html: string | null, consoleReplayScript: string, hasErrors: boolean, error: RenderingError | null): RenderResult { - const result: RenderResult = { html, consoleReplayScript, hasErrors }; - if (error) { - result.renderingError = { - message: error.message, - stack: error.stack, - }; - } - return result; -} - -function createSyncResult(renderState: RenderState & { result: string | null }, consoleReplayScript: string): RenderResult { - return createResultObject(renderState.result, consoleReplayScript, renderState.hasErrors, renderState.error); +function createResultObject(html: string | null, consoleReplayScript: string, hasErrors: boolean, error?: RenderingError): RenderResult { + return { + html, + consoleReplayScript, + hasErrors, + renderingError: error && { message: error.message, stack: error.stack }, + }; } -function createPromiseResult(renderState: RenderState & { result: Promise }, consoleReplayScript: string): Promise { - return (async () => { - try { - const html = await renderState.result; - return createResultObject(html, consoleReplayScript, renderState.hasErrors, renderState.error); - } catch (e: unknown) { - const error = e instanceof Error ? e : new Error(String(e)); - const html = handleError({ e: error, name: 'Unknown', serverSide: true }); - return createResultObject(html, consoleReplayScript, true, error); - } - })(); +async function createPromiseResult( + renderState: RenderState & { result: Promise }, + consoleReplayScript: string, + componentName: string, + throwJsErrors: boolean +): Promise { + try { + const html = await renderState.result; + return createResultObject(html, consoleReplayScript, renderState.hasErrors, renderState.error); + } catch (e: unknown) { + const errorRenderState = handleRenderingError(e, { componentName, throwJsErrors }); + return createResultObject(errorRenderState.result, consoleReplayScript, errorRenderState.hasErrors, errorRenderState.error); + } } -function createFinalResult(renderState: RenderState): null | string | Promise { +function createFinalResult( + renderState: RenderState, + componentName: string, + throwJsErrors: boolean +): null | string | Promise { const consoleReplayScript = buildConsoleReplay(); const { result } = renderState; if (isPromise(result)) { - return createPromiseResult({ ...renderState, result }, consoleReplayScript); + return createPromiseResult({ ...renderState, result }, consoleReplayScript, componentName, throwJsErrors); } - return JSON.stringify(createSyncResult({ ...renderState, result }, consoleReplayScript)); + return JSON.stringify(createResultObject(result, consoleReplayScript, renderState.hasErrors, renderState.error)); } function serverRenderReactComponentInternal(options: RenderParams): null | string | Promise { - const { name, domNodeId, trace, props, railsContext, renderingReturnsPromises, throwJsErrors } = options; + const { name: componentName, domNodeId, trace, props, railsContext, renderingReturnsPromises, throwJsErrors } = options; let renderState: RenderState = { result: null, hasErrors: false, - error: null, }; try { - const componentObj = ComponentRegistry.get(name); - validateComponent(componentObj, name); + const componentObj = ComponentRegistry.get(componentName); + validateComponent(componentObj, componentName); // Renders the component or executes the render function // - If the registered component is a React element or component, it renders it @@ -149,13 +151,14 @@ function serverRenderReactComponentInternal(options: RenderParams): null | strin // - For other values (e.g., strings), it returns them directly // Note: Only synchronous operations are performed at this stage const reactRenderingResult = createReactOutput({ componentObj, domNodeId, trace, props, railsContext }); + // Processes the result from createReactOutput: // 1. Converts React elements to HTML strings // 2. Returns rendered HTML from serverRenderHash // 3. Handles promises for async rendering - renderState.result = processRenderingResult(reactRenderingResult, { name, domNodeId, trace, renderingReturnsPromises }); - } catch (e) { - renderState = handleRenderingError(e as Error, renderState, { name, throwJsErrors }); + renderState = processRenderingResult(reactRenderingResult, { componentName, domNodeId, trace, renderingReturnsPromises }); + } catch (e: unknown) { + renderState = handleRenderingError(e, { componentName, throwJsErrors }); } // Finalize the rendering result and prepare it for server response @@ -167,7 +170,7 @@ function serverRenderReactComponentInternal(options: RenderParams): null | strin // - hasErrors: boolean (Indicates if any errors occurred during rendering) // - renderingError: Error | null (The error object if an error occurred, null otherwise) // 4. For Promise results, it awaits resolution before creating the final JSON - return createFinalResult(renderState); + return createFinalResult(renderState, componentName, throwJsErrors); } const serverRenderReactComponent: typeof serverRenderReactComponentInternal = (options) => { From 033fa74d70a421c45e330d309704a3436cd9b55f Mon Sep 17 00:00:00 2001 From: Abanoub Ghadban Date: Sun, 13 Oct 2024 15:16:54 +0300 Subject: [PATCH 4/9] don't install stream package --- spec/dummy/package.json | 1 - spec/dummy/yarn.lock | 14 +------------- 2 files changed, 1 insertion(+), 14 deletions(-) diff --git a/spec/dummy/package.json b/spec/dummy/package.json index f7b5c11d7..44a437380 100644 --- a/spec/dummy/package.json +++ b/spec/dummy/package.json @@ -52,7 +52,6 @@ "sass-loader": "^12.3.0", "sass-resources-loader": "^2.1.0", "shakapacker": "8.0.0", - "stream": "^0.0.3", "style-loader": "^3.3.1", "terser-webpack-plugin": "5.3.1", "url-loader": "^4.0.0", diff --git a/spec/dummy/yarn.lock b/spec/dummy/yarn.lock index 93bc35abc..1a634f47d 100644 --- a/spec/dummy/yarn.lock +++ b/spec/dummy/yarn.lock @@ -3375,11 +3375,6 @@ component-emitter@^1.2.1: resolved "https://registry.yarnpkg.com/component-emitter/-/component-emitter-1.3.0.tgz#16e4070fba8ae29b679f2215853ee181ab2eabc0" integrity sha512-Rd3se6QB+sO1TwqZjscQrurpEPIfO0/yYnSin6Q/rD3mOutHvUrCAhJub3r90uNb+SESBuE0QYoB90YdfatsRg== -component-emitter@^2.0.0: - version "2.0.0" - resolved "https://registry.yarnpkg.com/component-emitter/-/component-emitter-2.0.0.tgz#3a137dfe66fcf2efe3eab7cb7d5f51741b3620c6" - integrity sha512-4m5s3Me2xxlVKG9PkZpQqHQR7bgpnN7joDMJ4yvVkVXngjoITG76IaZmzmywSeRTeTpc6N6r3H3+KyUurV8OYw== - compose-function@3.0.3: version "3.0.3" resolved "https://registry.yarnpkg.com/compose-function/-/compose-function-3.0.3.tgz#9ed675f13cc54501d30950a486ff6a7ba3ab185f" @@ -6457,7 +6452,7 @@ react-is@^18.0.0: integrity sha512-xWGDIW6x921xtzPkhiULtthJHoJvBbF3q26fzloPCK0hsvxtPVelvftw3zjbHWSkR2km9Z+4uxbDDK/6Zw9B8w== "react-on-rails@file:.yalc/react-on-rails": - version "14.0.4" + version "14.0.0" dependencies: "@babel/runtime-corejs3" "^7.12.5" @@ -7300,13 +7295,6 @@ stream-http@^2.7.2: to-arraybuffer "^1.0.0" xtend "^4.0.0" -stream@^0.0.3: - version "0.0.3" - resolved "https://registry.yarnpkg.com/stream/-/stream-0.0.3.tgz#3f3934a900a561ce3e2b9ffbd2819cead32699d9" - integrity sha512-aMsbn7VKrl4A2T7QAQQbzgN7NVc70vgF5INQrBXqn4dCXN1zy3L9HGgLO5s7PExmdrzTJ8uR/27aviW8or8/+A== - dependencies: - component-emitter "^2.0.0" - string-width@^3.0.0, string-width@^3.1.0: version "3.1.0" resolved "https://registry.yarnpkg.com/string-width/-/string-width-3.1.0.tgz#22767be21b62af1081574306f69ac51b62203961" From 044cb13392f3a696bd5523e454a13bb8356dc5f3 Mon Sep 17 00:00:00 2001 From: Abanoub Ghadban Date: Sun, 13 Oct 2024 15:37:43 +0300 Subject: [PATCH 5/9] add a comment about building console replay before awaiting promise --- node_package/src/serverRenderReactComponent.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/node_package/src/serverRenderReactComponent.ts b/node_package/src/serverRenderReactComponent.ts index 8f6f365bf..573a87a63 100644 --- a/node_package/src/serverRenderReactComponent.ts +++ b/node_package/src/serverRenderReactComponent.ts @@ -121,6 +121,12 @@ function createFinalResult( componentName: string, throwJsErrors: boolean ): null | string | Promise { + // Node can handle multiple rendering requests simultaneously. + // Console history is stored globally in `console.history`. + // To prevent cross-request data leakage: + // 1. We build the consoleReplayScript here, before any async operations. + // 2. The console history is reset after the sync part of each request. + // This causes console logs happening during async operations to not be captured. const consoleReplayScript = buildConsoleReplay(); const { result } = renderState; @@ -163,7 +169,7 @@ function serverRenderReactComponentInternal(options: RenderParams): null | strin // Finalize the rendering result and prepare it for server response // 1. Builds the consoleReplayScript for client-side console replay - // 2. Handles both synchronous and asynchronous (Promise) results + // 2. Extract the result from promise (if needed) by awaiting it // 3. Constructs a JSON object with the following properties: // - html: string | null (The rendered component HTML) // - consoleReplayScript: string (Script to replay console outputs on the client) From b86acd19089d0484e9c015bb00f2445daf7864d8 Mon Sep 17 00:00:00 2001 From: Abanoub Ghadban Date: Sun, 13 Oct 2024 15:40:51 +0300 Subject: [PATCH 6/9] make a comment clearer --- node_package/src/serverRenderReactComponent.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/node_package/src/serverRenderReactComponent.ts b/node_package/src/serverRenderReactComponent.ts index 573a87a63..cd1d93bdf 100644 --- a/node_package/src/serverRenderReactComponent.ts +++ b/node_package/src/serverRenderReactComponent.ts @@ -121,11 +121,12 @@ function createFinalResult( componentName: string, throwJsErrors: boolean ): null | string | Promise { - // Node can handle multiple rendering requests simultaneously. // Console history is stored globally in `console.history`. - // To prevent cross-request data leakage: - // 1. We build the consoleReplayScript here, before any async operations. - // 2. The console history is reset after the sync part of each request. + // If node renderer is handling a render request that returns a promise, + // It can handle another request while awaiting the promise. + // To prevent cross-request console logs leakage between these requests, + // we build the consoleReplayScript before awaiting any promises. + // The console history is reset after the synchronous part of each request. // This causes console logs happening during async operations to not be captured. const consoleReplayScript = buildConsoleReplay(); From 45b61ba1553a5bfe75c17109d69d270a726d871a Mon Sep 17 00:00:00 2001 From: Abanoub Ghadban Date: Sun, 13 Oct 2024 15:47:48 +0300 Subject: [PATCH 7/9] add a comment --- node_package/src/serverRenderReactComponent.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/node_package/src/serverRenderReactComponent.ts b/node_package/src/serverRenderReactComponent.ts index cd1d93bdf..2e84aea45 100644 --- a/node_package/src/serverRenderReactComponent.ts +++ b/node_package/src/serverRenderReactComponent.ts @@ -41,6 +41,8 @@ function processServerRenderHash(result: ServerRenderResult, options: RenderOpti const redirectPath = redirectLocation.pathname + redirectLocation.search; console.log(`ROUTER REDIRECT: ${options.componentName} to dom node with id: ${options.domNodeId}, redirect to ${redirectPath}`); } + // For redirects on server rendering, we can't stop Rails from returning the same result. + // Possibly, someday, we could have the rails server redirect. htmlResult = ''; } else { htmlResult = result.renderedHtml as string; From cdbdc868cb2fa6fc4b5fcef97f287cb5f2b19226 Mon Sep 17 00:00:00 2001 From: Abanoub Ghadban Date: Mon, 14 Oct 2024 12:03:52 +0300 Subject: [PATCH 8/9] move some comments to another place --- .../src/serverRenderReactComponent.ts | 6 ++-- node_package/src/types/index.ts | 32 ++++++++++++++++--- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/node_package/src/serverRenderReactComponent.ts b/node_package/src/serverRenderReactComponent.ts index 2e84aea45..f54e926bc 100644 --- a/node_package/src/serverRenderReactComponent.ts +++ b/node_package/src/serverRenderReactComponent.ts @@ -42,7 +42,7 @@ function processServerRenderHash(result: ServerRenderResult, options: RenderOpti console.log(`ROUTER REDIRECT: ${options.componentName} to dom node with id: ${options.domNodeId}, redirect to ${redirectPath}`); } // For redirects on server rendering, we can't stop Rails from returning the same result. - // Possibly, someday, we could have the rails server redirect. + // Possibly, someday, we could have the Rails server redirect. htmlResult = ''; } else { htmlResult = result.renderedHtml as string; @@ -51,14 +51,14 @@ function processServerRenderHash(result: ServerRenderResult, options: RenderOpti return { result: htmlResult, hasErrors }; } -function processPromise(result: Promise, renderingReturnsPromises: boolean): Promise | string { +function processPromise(result: Promise, renderingReturnsPromises: boolean): Promise | string { if (!renderingReturnsPromises) { console.error('Your render function returned a Promise, which is only supported by a node renderer, not ExecJS.'); // If the app is using server rendering with ExecJS, then the promise will not be awaited. // And when a promise is passed to JSON.stringify, it will be converted to '{}'. return '{}'; } - return result as Promise; + return result; } function processReactElement(result: ReactElement): string { diff --git a/node_package/src/types/index.ts b/node_package/src/types/index.ts index 70bc03522..d2e129db5 100644 --- a/node_package/src/types/index.ts +++ b/node_package/src/types/index.ts @@ -43,6 +43,30 @@ type CreateReactOutputResult = ServerRenderResult | ReactElement | Promise; +/** + * Render functions are used to create dynamic React components or server-rendered HTML with side effects. + * They receive two arguments: props and railsContext. + * + * @param props - The component props passed to the render function + * @param railsContext - The Rails context object containing environment information + * @returns A string, React component, React element, or a Promise resolving to a string + * + * @remarks + * To distinguish a render function from a React Function Component: + * 1. Ensure it accepts two parameters (props and railsContext), even if railsContext is unused, or + * 2. Set the `renderFunction` property to `true` on the function object. + * + * If neither condition is met, it will be treated as a React Function Component, + * and ReactDOMServer will attempt to render it. + * + * @example + * // Option 1: Two-parameter function + * const renderFunction = (props, railsContext) => { ... }; + * + * // Option 2: Using renderFunction property + * const anotherRenderFunction = (props) => { ... }; + * anotherRenderFunction.renderFunction = true; + */ interface RenderFunction { (props?: any, railsContext?: RailsContext, domNodeId?: string): RenderFunctionResult; // We allow specifying that the function is RenderFunction and not a React Function Component @@ -67,10 +91,10 @@ export type { // eslint-disable-line import/prefer-default-export export interface RegisteredComponent { name: string; component: ReactComponentOrRenderFunction; - // Indicates if the registered component is a Render-function. - // Render-functions receive two arguments: props and railsContext. - // They should return a string, React component, React element, or a Promise. - // These functions are used to create dynamic React components or server-rendered HTML. + /** + * Indicates if the registered component is a RenderFunction + * @see RenderFunction for more details on its behavior and usage. + */ renderFunction: boolean; // Indicates if the registered component is a Renderer function. // Renderer function handles DOM rendering or hydration with 3 args: (props, railsContext, domNodeId) From d1d9d334c57c9950bc3d335c7720aae9180b1132 Mon Sep 17 00:00:00 2001 From: Abanoub Ghadban Date: Mon, 14 Oct 2024 19:07:30 +0300 Subject: [PATCH 9/9] make createResultObject takes renderState object --- node_package/src/serverRenderReactComponent.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/node_package/src/serverRenderReactComponent.ts b/node_package/src/serverRenderReactComponent.ts index f54e926bc..fec68e231 100644 --- a/node_package/src/serverRenderReactComponent.ts +++ b/node_package/src/serverRenderReactComponent.ts @@ -94,12 +94,12 @@ function handleRenderingError(e: unknown, options: { componentName: string, thro }; } -function createResultObject(html: string | null, consoleReplayScript: string, hasErrors: boolean, error?: RenderingError): RenderResult { +function createResultObject(html: string | null, consoleReplayScript: string, renderState: RenderState): RenderResult { return { html, consoleReplayScript, - hasErrors, - renderingError: error && { message: error.message, stack: error.stack }, + hasErrors: renderState.hasErrors, + renderingError: renderState.error && { message: renderState.error.message, stack: renderState.error.stack }, }; } @@ -111,10 +111,10 @@ async function createPromiseResult( ): Promise { try { const html = await renderState.result; - return createResultObject(html, consoleReplayScript, renderState.hasErrors, renderState.error); + return createResultObject(html, consoleReplayScript, renderState); } catch (e: unknown) { const errorRenderState = handleRenderingError(e, { componentName, throwJsErrors }); - return createResultObject(errorRenderState.result, consoleReplayScript, errorRenderState.hasErrors, errorRenderState.error); + return createResultObject(errorRenderState.result, consoleReplayScript, errorRenderState); } } @@ -137,7 +137,7 @@ function createFinalResult( return createPromiseResult({ ...renderState, result }, consoleReplayScript, componentName, throwJsErrors); } - return JSON.stringify(createResultObject(result, consoleReplayScript, renderState.hasErrors, renderState.error)); + return JSON.stringify(createResultObject(result, consoleReplayScript, renderState)); } function serverRenderReactComponentInternal(options: RenderParams): null | string | Promise {