Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor serverRenderReactComponent function #1653

6 changes: 3 additions & 3 deletions node_package/src/isServerRenderResult.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export function isServerRenderHash(testValue: CreateReactOutputResult):
(testValue as ServerRenderResult).error);
}

export function isPromise(testValue: CreateReactOutputResult):
testValue is Promise<string> {
return !!((testValue as Promise<string>).then);
export function isPromise<T>(testValue: CreateReactOutputResult | Promise<T> | string | null):
testValue is Promise<T> {
return !!((testValue as Promise<T> | null)?.then);
Comment on lines +12 to +14
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider renaming the function and improving promise detection.

The changes to the isPromise function improve its flexibility and type safety. However, there are a couple of points to consider:

  1. The function name isPromise might be misleading as it now accepts non-promise types. Consider renaming it to something like isPromiseLike or couldBePromise to better reflect its broader input types.

  2. The current implementation might not correctly identify all promise-like objects. The then property check is a good start, but it's not foolproof. Consider using a more robust check.

Here's a suggested improvement:

export function isPromiseLike<T>(testValue: CreateReactOutputResult | Promise<T> | string | null): testValue is Promise<T> {
  return testValue != null && typeof testValue === 'object' && typeof (testValue as any).then === 'function';
}

This implementation:

  • Renames the function to isPromiseLike.
  • Checks if testValue is not null and is an object.
  • Verifies that the then property is a function.

These changes make the function more accurate in identifying promise-like objects while maintaining its flexibility.

}
285 changes: 156 additions & 129 deletions node_package/src/serverRenderReactComponent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,154 +6,180 @@ 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<string>;
hasErrors: boolean;
error?: RenderingError;
};

function serverRenderReactComponentInternal(options: RenderParams): null | string | Promise<RenderResult> {
const { name, domNodeId, trace, props, railsContext, renderingReturnsPromises, throwJsErrors } = options;
type RenderOptions = {
componentName: string;
domNodeId?: string;
trace?: boolean;
renderingReturnsPromises: boolean;
};

let renderResult: null | string | Promise<string> = null;
let hasErrors = false;
let renderingError: null | RenderingError = null;
function validateComponent(componentObj: RegisteredComponent, componentName: string) {
if (componentObj.isRenderer) {
throw new Error(`Detected a renderer while server rendering component '${componentName}'. See https://github.com/shakacode/react_on_rails#renderer-functions`);
}
}
AbanoubGhadban marked this conversation as resolved.
Show resolved Hide resolved

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`);
function processServerRenderHash(result: ServerRenderResult, options: RenderOptions): RenderState {
const { redirectLocation, routeError } = result;
const hasErrors = !!routeError;

if (hasErrors) {
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.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;
}

return { result: htmlResult, hasErrors };
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

LGTM with suggestion: Comprehensive server render hash processing

The processServerRenderHash function handles various scenarios well, including redirects and errors. The trace logging is helpful for debugging.

Consider enhancing the error handling:

 if (hasErrors) {
-  console.error(`React Router ERROR: ${JSON.stringify(routeError)}`);
+  console.error('React Router ERROR:', routeError);
 }

This change avoids potential circular reference issues in JSON.stringify and provides a more readable error output.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function processServerRenderHash(result: ServerRenderResult, options: RenderOptions): RenderState {
const { redirectLocation, routeError } = result;
const hasErrors = !!routeError;
if (hasErrors) {
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.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;
}
return { result: htmlResult, hasErrors };
}
function processServerRenderHash(result: ServerRenderResult, options: RenderOptions): RenderState {
const { redirectLocation, routeError } = result;
const hasErrors = !!routeError;
if (hasErrors) {
console.error('React Router ERROR:', routeError);
}
let htmlResult: string;
if (redirectLocation) {
if (options.trace) {
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;
}
return { result: htmlResult, hasErrors };
}


function processPromise(result: Promise<string>, renderingReturnsPromises: boolean): Promise<string> | 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;
}

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;
};

const processPromise = () => {
if (!renderingReturnsPromises) {
console.error('Your render function returned a Promise, which is only supported by a node renderer, not ExecJS.');
}
return reactRenderingResult;
};

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<string>;
} else {
renderResult = processReactElement();
}
} catch (e: any) {
if (throwJsErrors) {
throw e;
}

hasErrors = true;
renderResult = handleError({
e,
name,
serverSide: true,
});
renderingError = e;
throw error;
}
}

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 processRenderingResult(result: CreateReactOutputResult, options: RenderOptions): RenderState {
if (isServerRenderHash(result)) {
return processServerRenderHash(result, options);
AbanoubGhadban marked this conversation as resolved.
Show resolved Hide resolved
}
if (isPromise(result)) {
return { result: processPromise(result, options.renderingReturnsPromises), hasErrors: false };
AbanoubGhadban marked this conversation as resolved.
Show resolved Hide resolved
}
return { result: processReactElement(result), hasErrors: false };
}

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;
};

return resolveRenderResult();
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 {
hasErrors: true,
result: handleError({ e: error, name: options.componentName, serverSide: true }),
error,
};
}

const result: RenderResult = {
html: renderResult as string,
function createResultObject(html: string | null, consoleReplayScript: string, hasErrors: boolean, error?: RenderingError): RenderResult {
AbanoubGhadban marked this conversation as resolved.
Show resolved Hide resolved
return {
html,
consoleReplayScript,
hasErrors,
renderingError: error && { message: error.message, stack: error.stack },
};
}

async function createPromiseResult(
renderState: RenderState & { result: Promise<string> },
consoleReplayScript: string,
componentName: string,
throwJsErrors: boolean
): Promise<RenderResult> {
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,
componentName: string,
throwJsErrors: boolean
): null | string | Promise<RenderResult> {
// Console history is stored globally in `console.history`.
// 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();

const { result } = renderState;
if (isPromise(result)) {
return createPromiseResult({ ...renderState, result }, consoleReplayScript, componentName, throwJsErrors);
}

return JSON.stringify(createResultObject(result, consoleReplayScript, renderState.hasErrors, renderState.error));
}

function serverRenderReactComponentInternal(options: RenderParams): null | string | Promise<RenderResult> {
const { name: componentName, domNodeId, trace, props, railsContext, renderingReturnsPromises, throwJsErrors } = options;

let renderState: RenderState = {
result: null,
hasErrors: false,
};

if (renderingError) {
addRenderingErrors(result, renderingError);
try {
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
// - 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 = processRenderingResult(reactRenderingResult, { componentName, domNodeId, trace, renderingReturnsPromises });
} catch (e: unknown) {
renderState = handleRenderingError(e, { componentName, 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. 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)
// - 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, componentName, throwJsErrors);
}

const serverRenderReactComponent: typeof serverRenderReactComponentInternal = (options) => {
Expand All @@ -165,4 +191,5 @@ const serverRenderReactComponent: typeof serverRenderReactComponentInternal = (o
console.history = [];
}
};

export default serverRenderReactComponent;
32 changes: 32 additions & 0 deletions node_package/src/types/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,30 @@ type CreateReactOutputResult = ServerRenderResult | ReactElement | Promise<strin

type RenderFunctionResult = ReactComponent | ServerRenderResult | Promise<string>;

/**
* 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
Expand All @@ -67,7 +91,15 @@ export type { // eslint-disable-line import/prefer-default-export
export interface RegisteredComponent {
name: string;
component: ReactComponentOrRenderFunction;
/**
* 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)
// Supported on the client side only.
// All renderer functions are render functions, but not all render functions are renderer functions.
isRenderer: boolean;
}

Expand Down
Loading