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.

Sign up for GitHub

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

STCOR-635 wrap pluggable in suspense #1213

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

STCOR-635 wrap pluggable in suspense #1213

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8601cfa
STCOR-635 wrap pluggable in suspense
zburke Jun 9, 2022
0395552
super helpful if you import the components you're using
zburke Jun 9, 2022
e8ddb8a
correctly import LoadingView
zburke Jun 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* Update NodeJS to v16 in GitHub Actions. Refs STCOR-623.
* Provide `useCallout` hook. Refs STCOR-631.
* Add message to indicate user cannot access app/record. Refs STCOR-619.
* Wrapp `<Pluggable>` in `<Suspense>` to avoid whole-tree re-render. Refs STCOR-635.

## [8.1.0](https://github.com/folio-org/stripes-core/tree/v8.1.0) (2022-02-11)
[Full Changelog](https://github.com/folio-org/stripes-core/compare/v8.0.0...v8.1.0)
Expand Down
12 changes: 9 additions & 3 deletions src/Pluggable.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React, { useMemo } from 'react';
import PropTypes from 'prop-types';
import { modules } from 'stripes-config';
import { withStripes } from './StripesContext';
import { ModuleHierarchyProvider } from './components';
import { LoadingView, ModuleHierarchyProvider } from './components';
Copy link
Contributor

Choose a reason for hiding this comment

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

@zburke I think <LoadingView> should come from @folio/stripes/components. This fixed my webpack error, but I didn't see what you reported in your comment


const Pluggable = (props) => {
const plugins = modules.plugin || [];
Expand Down Expand Up @@ -37,7 +37,9 @@ const Pluggable = (props) => {
if (cachedPlugins.length) {
return cachedPlugins.map(({ plugin, Child }) => (
<ModuleHierarchyProvider module={plugin}>
<Child {...props} actAs="plugin" />
<Suspense fallback={<LoadingView />}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<Suspense fallback={<LoadingView />}>
<Suspense fallback={<Loading />}>

<Child {...props} actAs="plugin" />
</Suspense>
</ModuleHierarchyProvider>
));
}
Expand All @@ -47,7 +49,11 @@ const Pluggable = (props) => {
// eslint-disable-next-line no-console
console.error(`<Pluggable type="${props.type}"> has ${props.children.length} children, can only return one`);
}
return props.children;
return (
<Suspense fallback={<LoadingView />}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<Suspense fallback={<LoadingView />}>
<Suspense fallback={<Loading />}>

{props.children}
</Suspense>
);
};

Pluggable.propTypes = {
Expand Down