Fix: Use React.lazy over async component imports for plugins

This commit is contained in:
Gustav Hansen 2023-08-17 08:48:59 +02:00
parent 2e1bda08f0
commit 4b69d56dbc
18 changed files with 151 additions and 237 deletions

View File

@ -1,4 +1,4 @@
import React from 'react';
import * as React from 'react';
import { darkTheme, lightTheme } from '@strapi/design-system';
import invariant from 'invariant';
@ -114,14 +114,30 @@ class StrapiApp {
);
invariant(
link.Component && typeof link.Component === 'function',
`link.Component should be a valid React Component`
`link.Component must be a function returning a Promise. Please use: \`Component: () => import(path)\` instead.`
);
invariant(
link.icon && typeof link.icon === 'function',
`link.Icon should be a valid React Component`
);
this.menu.push(link);
if (
link.Component &&
typeof link.Component === 'function' &&
link.Component[Symbol.toStringTag] === 'AsyncFunction'
) {
console.warn(`
[deprecated] addMenuLink() was called with an async Component from the plugin "${link.intlLabel.Internationalization}". This will be removed
in the future. Please use: \`Component: () => import(path)\` instead.
`);
}
this.menu.push({
...link,
// React.lazy can be removed once we migrate to react-router@6, because the <Route /> component can handle it natively
Component: React.lazy(link.Component),
});
};
addMiddlewares = (middlewares) => {
@ -149,10 +165,26 @@ class StrapiApp {
invariant(link.to, `link.to should be defined for ${stringifiedLink}`);
invariant(
link.Component && typeof link.Component === 'function',
`link.Component should be a valid React Component`
`link.Component must be a function returning a Promise. Please use: \`Component: () => import(path)\` instead.`
);
this.settings[sectionId].links.push(link);
if (
link.Component &&
typeof link.Component === 'function' &&
link.Component[Symbol.toStringTag] === 'AsyncFunction'
) {
console.warn(`
[deprecated] addSettingsLink() was called with an async Component from the plugin: "${link.intlLabel.Internationalization}". This will be removed
in the future. Please use: \`Component: () => import(path)\` instead.
`);
}
this.settings[sectionId].links.push({
...link,
// React.lazy can be removed once we migrate to react-router@6, because the <Route /> component can handle it natively
Component: React.lazy(link.Component),
});
};
addSettingsLinks = (sectionId, links) => {

View File

@ -111,7 +111,8 @@ Providers.propTypes = {
defaultMessage: PropTypes.string.isRequired,
}).isRequired,
permissions: PropTypes.array,
Component: PropTypes.func,
// React.lazy loadable
Component: PropTypes.object,
})
).isRequired,
menuLogo: PropTypes.oneOfType([PropTypes.string, PropTypes.any]).isRequired,

View File

@ -16,7 +16,6 @@ import { Route, Switch } from 'react-router-dom';
import LeftMenu from '../../components/LeftMenu';
import useConfigurations from '../../hooks/useConfigurations';
import useMenu from '../../hooks/useMenu';
import { createRoute } from '../../utils/createRoute';
import { SET_APP_RUNTIME_STATUS } from '../App/constants';
const CM = React.lazy(() =>
@ -76,25 +75,6 @@ export const Admin = () => {
}
}, [appStatus, dispatch, trackUsage]);
const routes = menu
.filter((link) => link.Component)
/**
* `Component` is an async function, which is passed as property of the
* addMenuLink() API during the plugin registration step.
*
* Because of that we can't just render <Route component={Component} />,
* but have to await the function.
*
* This isn't a good React pattern and should be reconsidered.
*/
.map(({ to, Component, exact }) => createRoute(Component, to, exact));
if (isLoading) {
return <LoadingIndicatorPage />;
}
return (
<DndProvider backend={HTML5Backend}>
<Flex alignItems="stretch">
@ -104,18 +84,31 @@ export const Admin = () => {
/>
<Box flex="1">
<React.Suspense fallback={<LoadingIndicatorPage />}>
{isLoading ? (
<LoadingIndicatorPage />
) : (
<Switch>
<Route path="/" component={HomePage} exact />
<Route path="/me" component={ProfilePage} exact />
<Route path="/content-manager" component={CM} />
{routes}
{menu.map(({ to, Component, exact }) => (
<Route
render={() => (
<React.Suspense fallback={<LoadingIndicatorPage />}>
<Component />
</React.Suspense>
)}
key={to}
path={to}
exact={exact || false}
/>
))}
<Route path="/settings/:settingId" component={SettingsPage} />
<Route path="/settings" component={SettingsPage} exact />
<Route path="/marketplace" component={MarketplacePage} />
<Route path="/list-plugins" component={InstalledPluginsPage} exact />
</Switch>
</React.Suspense>
)}
</Box>
{/* TODO: we should move the logic to determine whether the guided tour is displayed

View File

@ -2,67 +2,67 @@ import * as React from 'react';
export const SETTINGS_ROUTES_CE = [
{
component: React.lazy(() =>
Component: React.lazy(() =>
import(/* webpackChunkName: "admin-roles-list" */ './pages/Roles/ProtectedListPage')
),
path: '/settings/roles',
},
{
component: React.lazy(() =>
Component: React.lazy(() =>
import(/* webpackChunkName: "admin-edit-roles-page" */ './pages/Roles/CreatePage')
),
path: '/settings/roles/duplicate/:id',
},
{
component: React.lazy(() =>
Component: React.lazy(() =>
import(/* webpackChunkName: "admin-edit-roles-page" */ './pages/Roles/CreatePage')
),
path: '/settings/roles/new',
},
{
component: React.lazy(() =>
Component: React.lazy(() =>
import(/* webpackChunkName: "admin-edit-roles-page" */ './pages/Roles/ProtectedEditPage')
),
path: '/settings/roles/:id',
},
{
component: React.lazy(() =>
Component: React.lazy(() =>
import(/* webpackChunkName: "admin-users" */ './pages/Users/ProtectedListPage')
),
path: '/settings/users',
},
{
component: React.lazy(() =>
Component: React.lazy(() =>
import(/* webpackChunkName: "admin-edit-users" */ './pages/Users/ProtectedEditPage')
),
path: '/settings/users/:id',
},
{
component: React.lazy(() =>
Component: React.lazy(() =>
import(/* webpackChunkName: "webhook-edit-page" */ './pages/Webhooks/ProtectedCreateView')
),
path: '/settings/webhooks/create',
},
{
component: React.lazy(() =>
Component: React.lazy(() =>
import(/* webpackChunkName: "webhook-edit-page" */ './pages/Webhooks/ProtectedEditView')
),
path: '/settings/webhooks/:id',
},
{
component: React.lazy(() =>
Component: React.lazy(() =>
import(/* webpackChunkName: "webhook-list-page" */ './pages/Webhooks/ProtectedListView')
),
path: '/settings/webhooks',
},
{
component: React.lazy(() =>
Component: React.lazy(() =>
import(/* webpackChunkName: "api-tokens-list-page" */ './pages/ApiTokens/ProtectedListView')
),
path: '/settings/api-tokens',
},
{
component: React.lazy(() =>
Component: React.lazy(() =>
import(
/* webpackChunkName: "api-tokens-create-page" */ './pages/ApiTokens/ProtectedCreateView'
)
@ -70,13 +70,13 @@ export const SETTINGS_ROUTES_CE = [
path: '/settings/api-tokens/create',
},
{
component: React.lazy(() =>
Component: React.lazy(() =>
import(/* webpackChunkName: "api-tokens-edit-page" */ './pages/ApiTokens/ProtectedEditView')
),
path: '/settings/api-tokens/:id',
},
{
component: React.lazy(() =>
Component: React.lazy(() =>
import(
/* webpackChunkName: "transfer-tokens-create-page" */ './pages/TransferTokens/ProtectedCreateView'
)
@ -84,7 +84,7 @@ export const SETTINGS_ROUTES_CE = [
path: '/settings/transfer-tokens/create',
},
{
component: React.lazy(() =>
Component: React.lazy(() =>
import(
/* webpackChunkName: "transfer-tokens-list-page" */ './pages/TransferTokens/ProtectedListView'
)
@ -92,7 +92,7 @@ export const SETTINGS_ROUTES_CE = [
path: '/settings/transfer-tokens',
},
{
component: React.lazy(() =>
Component: React.lazy(() =>
import(
/* webpackChunkName: "transfer-tokens-edit-page" */ './pages/TransferTokens/ProtectedEditView'
)

View File

@ -8,7 +8,6 @@ import { Redirect, Route, Switch, useParams } from 'react-router-dom';
import { useSettingsMenu } from '../../hooks';
import { useEnterprise } from '../../hooks/useEnterprise';
import { createRoute } from '../../utils/createRoute';
import SettingsNav from './components/SettingsNav';
import { SETTINGS_ROUTES_CE } from './constants';
@ -31,26 +30,6 @@ export function SettingsPage() {
}
);
/**
* `Component` is an async function, which is passed as property of the
* addSettingsLink() API during the plugin bootstrap step.
*
* Because of that we can't just render <Route component={Component} />,
* but have to await the function.
*
* This isn't a good React pattern and should be reconsidered.
*/
const pluginSettingsRoutes = Object.values(settings).flatMap((section) =>
section.links.map((link) => createRoute(link.Component, link.to, link.exact || false))
);
// Since the useSettingsMenu hook can make API calls in order to check the links permissions
// We need to add a loading state to prevent redirecting the user while permissions are being checked
if (isLoading) {
return <LoadingIndicatorPage />;
}
if (!settingId) {
return <Redirect to="/settings/application-infos" />;
}
@ -64,15 +43,49 @@ export function SettingsPage() {
})}
/>
<Switch>
<Route path="/settings/application-infos" component={ApplicationInfosPage} exact />
{isLoading ? (
<LoadingIndicatorPage />
) : (
<Switch>
<Route
path="/settings/application-infos"
render={() => (
<React.Suspense fallback={<LoadingIndicatorPage />}>
<ApplicationInfosPage />
</React.Suspense>
)}
exact
/>
{routes.map(({ path, component }) => (
<Route key={path} path={path} component={component} exact />
))}
{routes.map(({ path, Component }) => (
<Route
key={path}
path={path}
render={() => (
<React.Suspense fallback={<LoadingIndicatorPage />}>
<Component />
</React.Suspense>
)}
exact
/>
))}
{pluginSettingsRoutes}
</Switch>
{Object.values(settings).flatMap((section) =>
section.links.map(({ Component, to, exact }) => (
<Route
render={() => (
<React.Suspense fallback={<LoadingIndicatorPage />}>
<Component />
</React.Suspense>
)}
key={to}
path={to}
exact={exact || false}
/>
))
)}
</Switch>
)}
</Layout>
);
}

View File

@ -9,11 +9,11 @@ import {
HeaderLayout,
Layout,
Link,
Loader,
Main,
Typography,
} from '@strapi/design-system';
import {
LoadingIndicatorPage,
prefixFileUrlWithBackendUrl,
SettingsPageTitle,
useAPIErrorHandler,
@ -161,12 +161,7 @@ const ApplicationInfosPage = () => {
<SettingsPageTitle name="Application" />
<Main>
{isLoading ? (
<Loader>
{formatMessage({
id: 'Settings.application.isLoading',
defaultMessage: 'Loading',
})}
</Loader>
<LoadingIndicatorPage data-testid="loader" />
) : (
<form onSubmit={handleSubmit}>
<HeaderLayout

View File

@ -256,7 +256,9 @@ describe('ADMIN | pages | SettingsPage', () => {
isDisplayed: true,
permissions: [],
to: '/settings/internationalization',
Component: () => ({ default: () => <div>i18n settings</div> }),
Component() {
return <div>i18n settings</div>;
},
},
],
},
@ -270,7 +272,9 @@ describe('ADMIN | pages | SettingsPage', () => {
isDisplayed: true,
permissions: [],
to: '/settings/email-settings',
Component: () => ({ default: () => <div>email settings</div> }),
Component() {
return <div>i18n settings</div>;
},
},
],
},
@ -292,7 +296,9 @@ describe('ADMIN | pages | SettingsPage', () => {
isDisplayed: true,
permissions: [],
to: '/settings/internationalization',
Component: () => ({ default: () => <div>i18n settings</div> }),
Component() {
return <div>i18n settings</div>;
},
},
],
},
@ -306,7 +312,9 @@ describe('ADMIN | pages | SettingsPage', () => {
isDisplayed: true,
permissions: [],
to: '/settings/email-settings',
Component: () => ({ default: () => <div>email settings</div> }),
Component() {
return <div>email settings</div>;
},
},
],
},

View File

@ -1,45 +0,0 @@
import React, { useEffect, useState } from 'react';
import { LoadingIndicatorPage } from '@strapi/helper-plugin';
import PropTypes from 'prop-types';
import { Route } from 'react-router-dom';
const LazyCompo = ({ loadComponent }) => {
const [Compo, setCompo] = useState(null);
useEffect(() => {
const loadCompo = async () => {
try {
const loadedCompo = await loadComponent();
setCompo(() => loadedCompo.default);
} catch (err) {
// TODO return the error component
console.log(err);
}
};
loadCompo();
}, [loadComponent]);
if (Compo) {
return <Compo />;
}
return <LoadingIndicatorPage />;
};
LazyCompo.propTypes = {
loadComponent: PropTypes.func.isRequired,
};
export const createRoute = (Component, to, exact) => {
return (
<Route
render={() => <LazyCompo loadComponent={Component} />}
key={to}
path={to}
exact={exact || false}
/>
);
};

View File

@ -1,16 +0,0 @@
import { createRoute } from '../createRoute';
describe('ADMIN | CONTAINER | SettingsPage | utils | createRoute', () => {
it('should return a <Route /> with the correctProps', () => {
const compo = () => 'test';
const {
props: { path, exact },
key,
} = createRoute(compo, '/test', true);
expect(key).toEqual('/test');
expect(exact).toBeTruthy();
expect(path).toEqual('/test');
});
});

View File

@ -4,7 +4,7 @@ export const SETTINGS_ROUTES_EE = [
...(window.strapi.features.isEnabled(window.strapi.features.AUDIT_LOGS)
? [
{
component: React.lazy(() =>
Component: React.lazy(() =>
import(
/* webpackChunkName: "audit-logs-settings-page" */ './pages/AuditLogs/ProtectedListPage'
)
@ -17,7 +17,7 @@ export const SETTINGS_ROUTES_EE = [
...(window.strapi.features.isEnabled(window.strapi.features.REVIEW_WORKFLOWS)
? [
{
component: React.lazy(() =>
Component: React.lazy(() =>
import(
/* webpackChunkName: "review-workflows-settings-list-view" */ './pages/ReviewWorkflows/pages/ListView'
)
@ -26,7 +26,7 @@ export const SETTINGS_ROUTES_EE = [
},
{
component: React.lazy(() =>
Component: React.lazy(() =>
import(
/* webpackChunkName: "review-workflows-settings-create-view" */ './pages/ReviewWorkflows/pages/CreateView'
)
@ -35,7 +35,7 @@ export const SETTINGS_ROUTES_EE = [
},
{
component: React.lazy(() =>
Component: React.lazy(() =>
import(
/* webpackChunkName: "review-workflows-settings-edit-view" */ './pages/ReviewWorkflows/pages/EditView'
)
@ -48,7 +48,7 @@ export const SETTINGS_ROUTES_EE = [
...(window.strapi.features.isEnabled(window.strapi.features.SSO)
? [
{
component: React.lazy(() =>
Component: React.lazy(() =>
import(/* webpackChunkName: "sso-settings-page" */ './pages/SingleSignOn')
),
path: '/settings/single-sign-on',

View File

@ -22,13 +22,7 @@ export default {
defaultMessage: 'Content Types Builder',
},
permissions: PERMISSIONS.main,
async Component() {
const component = await import(
/* webpackChunkName: "content-type-builder" */ './pages/App'
);
return component;
},
Component: () => import(/* webpackChunkName: "content-type-builder" */ './pages/App'),
});
app.registerPlugin({

View File

@ -1,10 +1,3 @@
// NOTE TO PLUGINS DEVELOPERS:
// If you modify this file by adding new options to the plugin entry point
// Here's the file: strapi/docs/3.x/plugin-development/frontend-field-api.md
// Here's the file: strapi/docs/3.x/guides/registering-a-field-in-admin.md
// Also the strapi-generate-plugins/files/admin/src/index.js needs to be updated
// IF THE DOC IS NOT UPDATED THE PULL REQUEST WILL NOT BE MERGED
import { prefixPluginTranslations } from '@strapi/helper-plugin';
import { PERMISSIONS } from './constants';
@ -25,17 +18,12 @@ export default {
},
id: 'settings',
to: `/settings/email`,
async Component() {
const component = await import(
/* webpackChunkName: "email-settings-page" */ './pages/Settings'
);
return component;
},
Component: () => import(/* webpackChunkName: "email-settings-page" */ './pages/Settings'),
permissions: PERMISSIONS.settings,
},
]
);
app.registerPlugin({
id: 'email',
name: 'email',

View File

@ -83,7 +83,8 @@ StrapiAppProvider.propTypes = {
defaultMessage: PropTypes.string.isRequired,
}).isRequired,
permissions: PropTypes.array,
Component: PropTypes.func,
// React.lazy loadable
Component: PropTypes.object,
})
).isRequired,
plugins: PropTypes.object.isRequired,

View File

@ -107,3 +107,4 @@ export { setHexOpacity } from './utils/setHexOpacity';
export * from './utils/stopPropagation';
export { translatedErrors } from './utils/translatedErrors';
export { wrapAxiosInstance } from './utils/wrapAxiosInstance';
export * from './utils/once';

View File

@ -1,9 +1,3 @@
// NOTE TO PLUGINS DEVELOPERS:
// If you modify this file by adding new options to the plugin entry point
// Here's the file: strapi/docs/3.0.0-beta.x/plugin-development/frontend-field-api.md
// Here's the file: strapi/docs/3.0.0-beta.x/guides/registering-a-field-in-admin.md
// Also the strapi-generate-plugins/files/admin/src/index.js needs to be updated
// IF THE DOC IS NOT UPDATED THE PULL REQUEST WILL NOT BE MERGED
import { prefixPluginTranslations } from '@strapi/helper-plugin';
import pluginPkg from '../../package.json';
@ -27,11 +21,7 @@ export default {
defaultMessage: 'Media Library',
},
permissions: PERMISSIONS.main,
async Component() {
const component = await import(/* webpackChunkName: "upload" */ './pages/App');
return component;
},
Component: () => import(/* webpackChunkName: "upload" */ './pages/App'),
});
app.addFields({ type: 'media', Component: MediaLibraryInput });
@ -50,13 +40,7 @@ export default {
defaultMessage: 'Media Library',
},
to: '/settings/media-library',
async Component() {
const component = await import(
/* webpackChunkName: "upload-settings" */ './pages/SettingsPage'
);
return component;
},
Component: () => import(/* webpackChunkName: "upload-settings" */ './pages/SettingsPage'),
permissions: PERMISSIONS.settings,
});
},

View File

@ -24,13 +24,7 @@ export default {
defaultMessage: 'Documentation',
},
permissions: PERMISSIONS.main,
async Component() {
const component = await import(
/* webpackChunkName: "documentation-page" */ './pages/PluginPage'
);
return component;
},
Component: () => import(/* webpackChunkName: "documentation-page" */ './pages/PluginPage'),
});
app.registerPlugin({
@ -46,13 +40,8 @@ export default {
},
id: 'documentation',
to: `/settings/${pluginId}`,
async Component() {
const component = await import(
/* webpackChunkName: "documentation-settings" */ './pages/SettingsPage'
);
return component;
},
Component: () =>
import(/* webpackChunkName: "documentation-settings" */ './pages/SettingsPage'),
permissions: PERMISSIONS.main,
});
},

View File

@ -61,13 +61,7 @@ export default {
id: 'internationalization',
to: '/settings/internationalization',
async Component() {
const component = await import(
/* webpackChunkName: "i18n-settings-page" */ './pages/SettingsPage'
);
return component;
},
Component: () => import(/* webpackChunkName: "i18n-settings-page" */ './pages/SettingsPage'),
permissions: PERMISSIONS.accessMain,
});

View File

@ -32,13 +32,8 @@ export default {
},
id: 'roles',
to: `/settings/users-permissions/roles`,
async Component() {
const component = await import(
/* webpackChunkName: "users-roles-settings-page" */ './pages/Roles'
);
return component;
},
Component: () =>
import(/* webpackChunkName: "users-roles-settings-page" */ './pages/Roles'),
permissions: PERMISSIONS.accessRoles,
},
{
@ -48,13 +43,8 @@ export default {
},
id: 'providers',
to: `/settings/users-permissions/providers`,
async Component() {
const component = await import(
/* webpackChunkName: "users-providers-settings-page" */ './pages/Providers'
);
return component;
},
Component: () =>
import(/* webpackChunkName: "users-providers-settings-page" */ './pages/Providers'),
permissions: PERMISSIONS.readProviders,
},
{
@ -64,13 +54,8 @@ export default {
},
id: 'email-templates',
to: `/settings/users-permissions/email-templates`,
async Component() {
const component = await import(
/* webpackChunkName: "users-email-settings-page" */ './pages/EmailTemplates'
);
return component;
},
Component: () =>
import(/* webpackChunkName: "users-email-settings-page" */ './pages/EmailTemplates'),
permissions: PERMISSIONS.readEmailTemplates,
},
{
@ -80,13 +65,10 @@ export default {
},
id: 'advanced-settings',
to: `/settings/users-permissions/advanced-settings`,
async Component() {
const component = await import(
Component: () =>
import(
/* webpackChunkName: "users-advanced-settings-page" */ './pages/AdvancedSettings'
);
return component;
},
),
permissions: PERMISSIONS.readAdvancedSettings,
},
]