Skip to content

Commit

Permalink
[sitecore-jss-react][sitecore-jss-nextjs] Prevent hydration error wit…
Browse files Browse the repository at this point in the history
…h dynamic components (#1807)

* [sitecore-jss-react][sitecore-jss-nextjs] Prevent hydration error with dynamic components

* support for custom error objects

* Support for both jss and non-jss dynamic implementations.

* add unit tests

* changelog
  • Loading branch information
art-alexeyenko authored Jun 3, 2024
1 parent 507800c commit 5a3dd9c
Show file tree
Hide file tree
Showing 8 changed files with 91 additions and 32 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ Our versioning strategy is as follows:

### 🎉 New Features & Improvements

* `[sitecore-jss-react]` Introduce ErrorBoundary component. All rendered components are wrapped with it and it will catch client or server side errors from any of its children, display appropriate message and prevent the rest of the application from failing. It accepts and can display custom error component and loading message if it is passed as a prop to parent Placeholder. ([#1786](https://github.com/Sitecore/jss/pull/1786))([#1790](https://github.com/Sitecore/jss/pull/1790))([#1793](https://github.com/Sitecore/jss/pull/1793))([#1794](https://github.com/Sitecore/jss/pull/1794))([#1799](https://github.com/Sitecore/jss/pull/1799))
* `[sitecore-jss-react]` Introduce ErrorBoundary component. All rendered components are wrapped with it and it will catch client or server side errors from any of its children, display appropriate message and prevent the rest of the application from failing. It accepts and can display custom error component and loading message if it is passed as a prop to parent Placeholder. ([#1786](https://github.com/Sitecore/jss/pull/1786))([#1790](https://github.com/Sitecore/jss/pull/1790))([#1793](https://github.com/Sitecore/jss/pull/1793))([#1794](https://github.com/Sitecore/jss/pull/1794))([#1799](https://github.com/Sitecore/jss/pull/1799))([#1807](https://github.com/Sitecore/jss/pull/1807))
* `[sitecore-jss-nextjs]` Enforce CORS policy that matches Sitecore Pages domains for editing middleware API endpoints ([#1798](https://github.com/Sitecore/jss/pull/1798)[#1801](https://github.com/Sitecore/jss/pull/1801))
* `[sitecore-jss]` _GraphQLRequestClient_ now can accept custom 'headers' in the constructor or via _createClientFactory_ ([#1806](https://github.com/Sitecore/jss/pull/1806))
* `[templates/nextjs]` Removed cors header for API endpoints from _lib/next-config/plugins/cors-header_ plugin since cors is handled by API handlers / middlewares ([#1806](https://github.com/Sitecore/jss/pull/1806))
Expand Down
9 changes: 4 additions & 5 deletions packages/sitecore-jss-nextjs/src/ComponentBuilder.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,18 @@
import { ComponentFactory } from '@sitecore-jss/sitecore-jss-react';
import { ComponentFactory, JssComponentType } from '@sitecore-jss/sitecore-jss-react';
import { Module, ModuleFactory } from './sharedTypes/module-factory';
import { ComponentType } from 'react';

/**
* Represents a component that can be imported dynamically
*/
export type LazyModule = {
module: () => Promise<Module>;
element: (isEditing?: boolean) => ComponentType;
element: (isEditing?: boolean) => JssComponentType;
};

/**
* Component is a module or a lazy module
*/
type Component = Module | LazyModule | ComponentType;
type Component = Module | LazyModule | JssComponentType;

/**
* Configuration for ComponentBuilder
Expand Down Expand Up @@ -100,7 +99,7 @@ export class ComponentBuilder {
return (
(component as Module).Default ||
(component as Module).default ||
(component as ComponentType)
(component as JssComponentType)
);
};
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React from 'react';
import React, { Suspense } from 'react';
import { expect } from 'chai';
import { mount } from 'enzyme';
import { spy } from 'sinon';
Expand Down Expand Up @@ -243,6 +243,50 @@ describe('ErrorBoundary', () => {
});
});
describe('when not in page editing and not in development mode', () => {
const delay = (timeout, promise?) => {
return new Promise((resolve) => {
setTimeout(resolve, timeout);
}).then(() => promise);
};

const ItsADynamicComponent = React.lazy(() =>
delay(500, import('../test-data/test-dynamic-component'))
);

it('should render a loading message', async () => {
const rendered = mount(
<ErrorBoundary>
<ItsADynamicComponent />
</ErrorBoundary>
);
expect(rendered.text()).to.equal('Loading component...');
});

it('should render custom loading message', async () => {
const loading = 'I am customly loading...';
const rendered = mount(
<ErrorBoundary componentLoadingMessage={loading}>
<ItsADynamicComponent />
</ErrorBoundary>
);
expect(rendered.text()).to.equal(loading);
});

it('should not render Suspense and default loading message when wrapping a dynamic component', async () => {
// mount fails with lazy component and no suspense
const rendered = mount(
<Suspense>
<ErrorBoundary isDynamic={true}>
<ItsADynamicComponent />
</ErrorBoundary>
</Suspense>
);
expect(rendered.text()).to.equal('');
await delay(500);
rendered.update();
expect(rendered.text()).to.equal('No error');
});

it('Should render custom error component when custom error component is provided and error is thrown', () => {
const errorMessage = 'an error occured';
const TestErrorComponent: React.FC = () => {
Expand Down
8 changes: 7 additions & 1 deletion packages/sitecore-jss-react/src/components/ErrorBoundary.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export type ErrorBoundaryProps = {
children: ReactNode;
sitecoreContext: SitecoreContextValue;
type: string;
isDynamic?: boolean;
errorComponent?: React.ComponentClass<ErrorComponentProps> | React.FC<ErrorComponentProps>;
rendering?: ComponentRendering;
componentLoadingMessage?: string;
Expand Down Expand Up @@ -64,7 +65,7 @@ class ErrorBoundary extends React.Component<ErrorBoundaryProps> {
A rendering error occurred in component{' '}
<em>{this.props.rendering?.componentName}</em>
<br />
Error: <em>{this.state.error.message}</em>
Error: <em>{this.state.error.message || JSON.stringify(this.state.error)}</em>
</div>
</div>
);
Expand All @@ -78,6 +79,11 @@ class ErrorBoundary extends React.Component<ErrorBoundaryProps> {
}
}

// do not apply suspense on already dynamic components
if (this.props.isDynamic) {
return this.props.children;
}

return (
<Suspense
fallback={<h4>{this.props.componentLoadingMessage || this.defaultLoadingMessage}</h4>}
Expand Down
42 changes: 19 additions & 23 deletions packages/sitecore-jss-react/src/components/PlaceholderCommon.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React, { ComponentType } from 'react';
import PropTypes, { Requireable } from 'prop-types';
import { MissingComponent } from './MissingComponent';
import { ComponentFactory } from './sharedTypes';
import { ComponentFactory, JssComponentType } from './sharedTypes';
import {
ComponentRendering,
RouteData,
Expand Down Expand Up @@ -267,11 +267,27 @@ export class PlaceholderCommon<T extends PlaceholderProps> extends React.Compone
rendering: componentRendering,
};

const rendered = React.createElement<{ [attr: string]: unknown }>(
let rendered = React.createElement<{ [attr: string]: unknown }>(
component as React.ComponentType,
this.props.modifyComponentProps ? this.props.modifyComponentProps(finalProps) : finalProps
);

const type = rendered.props.type === 'text/sitecore' ? rendered.props.type : '';
if (!isEmpty) {
rendered = (
<ErrorBoundary
key={rendered.type + '-' + index}
errorComponent={this.props.errorComponent}
componentLoadingMessage={this.props.componentLoadingMessage}
type={type}
isDynamic={(component as JssComponentType).render?.preload ? true : false}
{...rendered.props}
>
{rendered}
</ErrorBoundary>
);
}

// if editMode is equal to 'metadata' then emit shallow chromes for hydration in Pages
if (this.props.sitecoreContext?.editMode === EditMode.Metadata) {
return (
Expand All @@ -297,27 +313,7 @@ export class PlaceholderCommon<T extends PlaceholderProps> extends React.Compone
];
}

if (isEmpty) {
return transformedComponents;
}

return transformedComponents.map((element, id) => {
// assign type based on passed element - type='text/sitecore' should be ignored when renderEach Placeholder prop function is being used
const type = element.props.type === 'text/sitecore' ? element.props.type : '';
return (
// wrapping with error boundary could cause problems in case where parent component uses withPlaceholder HOC and tries to access its children props
// that's why we need to expose element's props here
<ErrorBoundary
key={element.type + '-' + id}
errorComponent={this.props.errorComponent}
componentLoadingMessage={this.props.componentLoadingMessage}
type={type}
{...element.props}
>
{element}
</ErrorBoundary>
);
});
return transformedComponents;
}

getComponentForRendering(renderingDefinition: ComponentRendering): ComponentType | null {
Expand Down
8 changes: 8 additions & 0 deletions packages/sitecore-jss-react/src/components/sharedTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,11 @@ import { ComponentType } from 'react';
* @param {string?} exportName component to be imported in case you export multiple components from the same file
*/
export type ComponentFactory = (componentName: string, exportName?: string) => ComponentType | null;

/**
* Component type returned from component builder / factory
*/
export type JssComponentType = ComponentType & {
// all dynamic elements will have a separate render prop
render?: { [key: string]: unknown };
};
2 changes: 1 addition & 1 deletion packages/sitecore-jss-react/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export {
GraphQLRequestClient,
} from '@sitecore-jss/sitecore-jss/graphql';
export { mediaApi } from '@sitecore-jss/sitecore-jss/media';
export { ComponentFactory } from './components/sharedTypes';
export { ComponentFactory, JssComponentType } from './components/sharedTypes';
export { Placeholder, PlaceholderComponentProps } from './components/Placeholder';
export {
Image,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import React from 'react';

// Import me into lazy components
const TestDynamicComponent = () => <div>No error</div>;

export default TestDynamicComponent;

0 comments on commit 5a3dd9c

Please sign in to comment.