From e43368e10e48d87803fce3f70dc793871a5de598 Mon Sep 17 00:00:00 2001 From: Gustav Hansen Date: Tue, 15 Aug 2023 14:14:13 +0200 Subject: [PATCH] Chore: Dissolve AppLayout and PluginInitializer --- .../admin/src/components/AuthenticatedApp.js | 82 ++++++++++++++++++- .../components/PluginsInitializer/index.js | 66 --------------- .../src/components/PluginsInitializer/init.js | 11 --- .../components/PluginsInitializer/reducer.js | 22 ----- .../PluginsInitializer/tests/index.test.js | 32 -------- .../PluginsInitializer/tests/init.test.js | 16 ---- .../PluginsInitializer/tests/reducer.test.js | 40 --------- .../admin/src/layouts/AppLayout/index.js | 33 -------- .../core/admin/admin/src/pages/Admin/index.js | 58 ++++++------- 9 files changed, 107 insertions(+), 253 deletions(-) delete mode 100644 packages/core/admin/admin/src/components/PluginsInitializer/index.js delete mode 100644 packages/core/admin/admin/src/components/PluginsInitializer/init.js delete mode 100644 packages/core/admin/admin/src/components/PluginsInitializer/reducer.js delete mode 100644 packages/core/admin/admin/src/components/PluginsInitializer/tests/index.test.js delete mode 100644 packages/core/admin/admin/src/components/PluginsInitializer/tests/init.test.js delete mode 100644 packages/core/admin/admin/src/components/PluginsInitializer/tests/reducer.test.js delete mode 100644 packages/core/admin/admin/src/layouts/AppLayout/index.js diff --git a/packages/core/admin/admin/src/components/AuthenticatedApp.js b/packages/core/admin/admin/src/components/AuthenticatedApp.js index c258f823f4..b72192843e 100644 --- a/packages/core/admin/admin/src/components/AuthenticatedApp.js +++ b/packages/core/admin/admin/src/components/AuthenticatedApp.js @@ -7,20 +7,43 @@ import { useFetchClient, useGuidedTour, useNotification, + useStrapiApp, } from '@strapi/helper-plugin'; +import produce from 'immer'; import { useQueries } from 'react-query'; import { valid, lt } from 'semver'; import packageJSON from '../../../package.json'; import { useConfigurations } from '../hooks'; +import { Admin } from '../pages/Admin'; import getFullName from '../utils/getFullName'; import { hashAdminUserEmail } from '../utils/uniqueAdminHash'; -import { PluginsInitializer } from './PluginsInitializer'; import RBACProvider from './RBACProvider'; const strapiVersion = packageJSON.version; +const initialState = { + plugins: null, +}; + +const reducer = (state = initialState, action) => + /* eslint-disable-next-line consistent-return */ + produce(state, (draftState) => { + switch (action.type) { + case 'SET_PLUGIN_READY': { + if (!draftState.plugins?.[action.pluginId]) { + draftState.plugins[action.pluginId] = {}; + } + + draftState.plugins[action.pluginId].isReady = true; + break; + } + default: + return draftState; + } + }); + const checkLatestStrapiVersion = (currentPackageVersion, latestPublishedVersion) => { if (!valid(currentPackageVersion) || !valid(latestPublishedVersion)) { return false; @@ -39,6 +62,13 @@ export const AuthenticatedApp = () => { const [userDisplayName, setUserDisplayName] = React.useState(userName); const [userId, setUserId] = React.useState(null); const { showReleaseNotification } = useConfigurations(); + const { plugins: appPlugins } = useStrapiApp(); + const [{ plugins }, dispatch] = React.useReducer(reducer, initialState, () => ({ + plugins: appPlugins, + })); + const setPlugin = React.useRef((pluginId) => { + dispatch({ type: 'SET_PLUGIN_READY', pluginId }); + }); const [ { data: appInfos, isLoading: isLoadingAppInfos }, { data: tagName, isLoading: isLoadingRelease }, @@ -135,8 +165,52 @@ export const AuthenticatedApp = () => { generateUserId(); }, [userInfo]); - if (isLoadingRelease || isLoadingAppInfos || isLoadingPermissions) { - return ; + const hasApluginNotReady = Object.keys(plugins).some( + (plugin) => plugins[plugin].isReady === false + ); + + /** + * + * I have spent some time trying to understand what is happening here, and wanted to + * leave that knowledge for my future me: + * + * `initializer` is an undocumented property of the `registerPlugin` API. At the time + * of writing it seems only to be used by the i18n plugin. + * + * How does it work? + * + * Every plugin that has an `initializer` component defined, receives the + * `setPlugin` function as a component prop. In the case of i18n the plugin fetches locales + * first and calls `setPlugin` with `pluginId` once they are loaded, which then triggers the + * reducer of the admin app defined above. + * + * Once all plugins are set to `isReady: true` the app renders. + * + * This API is used to block rendering of the admin app. We should remove that in v5 completely + * and make sure plugins can inject data into the global store before they are initialized, to avoid + * having a new prop-callback based communication channel between plugins and the core admin app. + * + */ + + if (isLoadingRelease || isLoadingAppInfos || isLoadingPermissions || hasApluginNotReady) { + const initializers = Object.keys(plugins).reduce((acc, current) => { + const InitializerComponent = plugins[current].initializer; + + if (InitializerComponent) { + const key = plugins[current].pluginId; + + acc.push(); + } + + return acc; + }, []); + + return ( + <> + {initializers} + + + ); } return ( @@ -149,7 +223,7 @@ export const AuthenticatedApp = () => { userDisplayName={userDisplayName} > - + ); diff --git a/packages/core/admin/admin/src/components/PluginsInitializer/index.js b/packages/core/admin/admin/src/components/PluginsInitializer/index.js deleted file mode 100644 index 26127a2b9c..0000000000 --- a/packages/core/admin/admin/src/components/PluginsInitializer/index.js +++ /dev/null @@ -1,66 +0,0 @@ -import * as React from 'react'; - -import { LoadingIndicatorPage, useStrapiApp } from '@strapi/helper-plugin'; - -import { Admin } from '../../pages/Admin'; - -import init from './init'; -import reducer, { initialState } from './reducer'; - -export const PluginsInitializer = () => { - const { plugins: appPlugins } = useStrapiApp(); - const [{ plugins }, dispatch] = React.useReducer(reducer, initialState, () => init(appPlugins)); - const setPlugin = React.useRef((pluginId) => { - dispatch({ type: 'SET_PLUGIN_READY', pluginId }); - }); - - const hasApluginNotReady = Object.keys(plugins).some( - (plugin) => plugins[plugin].isReady === false - ); - - /** - * - * I have spent some time trying to understand what is happening here, and wanted to - * leave that knowledge for my future me: - * - * `initializer` is an undocumented property of the `registerPlugin` API. At the time - * of writing it seems only to be used by the i18n plugin. - * - * How does it work? - * - * Every plugin that has an `initializer` component defined, receives the - * `setPlugin` function as a component prop. In the case of i18n the plugin fetches locales - * first and calls `setPlugin` with `pluginId` once they are loaded, which then triggers the - * reducer of the admin app defined above. - * - * Once all plugins are set to `isReady: true` the app renders. - * - * This API is used to block rendering of the admin app. We should remove that in v5 completely - * and make sure plugins can inject data into the global store before they are initialized, to avoid - * having a new prop-callback based communication channel between plugins and the core admin app. - * - */ - - if (hasApluginNotReady) { - const initializers = Object.keys(plugins).reduce((acc, current) => { - const InitializerComponent = plugins[current].initializer; - - if (InitializerComponent) { - const key = plugins[current].pluginId; - - acc.push(); - } - - return acc; - }, []); - - return ( - <> - {initializers} - - - ); - } - - return ; -}; diff --git a/packages/core/admin/admin/src/components/PluginsInitializer/init.js b/packages/core/admin/admin/src/components/PluginsInitializer/init.js deleted file mode 100644 index c73c65fa20..0000000000 --- a/packages/core/admin/admin/src/components/PluginsInitializer/init.js +++ /dev/null @@ -1,11 +0,0 @@ -const init = (plugins) => { - return { - plugins: Object.keys(plugins).reduce((acc, current) => { - acc[current] = { ...plugins[current] }; - - return acc; - }, {}), - }; -}; - -export default init; diff --git a/packages/core/admin/admin/src/components/PluginsInitializer/reducer.js b/packages/core/admin/admin/src/components/PluginsInitializer/reducer.js deleted file mode 100644 index eb5826edc9..0000000000 --- a/packages/core/admin/admin/src/components/PluginsInitializer/reducer.js +++ /dev/null @@ -1,22 +0,0 @@ -import produce from 'immer'; -import set from 'lodash/set'; - -const initialState = { - plugins: null, -}; - -const reducer = (state = initialState, action) => - /* eslint-disable-next-line consistent-return */ - produce(state, (draftState) => { - switch (action.type) { - case 'SET_PLUGIN_READY': { - set(draftState, ['plugins', action.pluginId, 'isReady'], true); - break; - } - default: - return draftState; - } - }); - -export { initialState }; -export default reducer; diff --git a/packages/core/admin/admin/src/components/PluginsInitializer/tests/index.test.js b/packages/core/admin/admin/src/components/PluginsInitializer/tests/index.test.js deleted file mode 100644 index 79ec9caf3d..0000000000 --- a/packages/core/admin/admin/src/components/PluginsInitializer/tests/index.test.js +++ /dev/null @@ -1,32 +0,0 @@ -import React from 'react'; - -import { StrapiAppProvider } from '@strapi/helper-plugin'; -import { render } from '@testing-library/react'; - -import { PluginsInitializer } from '../index'; - -jest.mock('../../../pages/Admin', () => () => { - return
ADMIN
; -}); - -describe('ADMIN | COMPONENTS | PluginsInitializer', () => { - it('should not crash', () => { - const getPlugin = jest.fn(); - - expect( - render( - - - - ) - ); - }); -}); diff --git a/packages/core/admin/admin/src/components/PluginsInitializer/tests/init.test.js b/packages/core/admin/admin/src/components/PluginsInitializer/tests/init.test.js deleted file mode 100644 index 89ba385879..0000000000 --- a/packages/core/admin/admin/src/components/PluginsInitializer/tests/init.test.js +++ /dev/null @@ -1,16 +0,0 @@ -import init from '../init'; - -describe('ADMIN | COMPONENT | PluginsInitializer | init', () => { - it('should return the initialState', () => { - const plugins = { - pluginA: { - isReady: false, - }, - pluginB: { - isReady: false, - }, - }; - - expect(init(plugins)).toEqual({ plugins }); - }); -}); diff --git a/packages/core/admin/admin/src/components/PluginsInitializer/tests/reducer.test.js b/packages/core/admin/admin/src/components/PluginsInitializer/tests/reducer.test.js deleted file mode 100644 index 808ebce395..0000000000 --- a/packages/core/admin/admin/src/components/PluginsInitializer/tests/reducer.test.js +++ /dev/null @@ -1,40 +0,0 @@ -import reducer, { initialState } from '../reducer'; - -describe('ADMIN | COMPONENTS | PluginsInitializer | reducer', () => { - let state; - - beforeEach(() => { - state = initialState; - }); - - describe('DEFAULT_ACTION', () => { - it('should return the initialState', () => { - expect(reducer(state, {})).toEqual(initialState); - }); - }); - - describe('SET_PLUGIN_READY', () => { - it('should set the isReady property to true for a plugin', () => { - state = { - plugins: { - pluginA: { - isReady: false, - }, - }, - }; - - const expected = { - plugins: { - pluginA: { isReady: true }, - }, - }; - - const action = { - type: 'SET_PLUGIN_READY', - pluginId: 'pluginA', - }; - - expect(reducer(state, action)).toEqual(expected); - }); - }); -}); diff --git a/packages/core/admin/admin/src/layouts/AppLayout/index.js b/packages/core/admin/admin/src/layouts/AppLayout/index.js deleted file mode 100644 index e8b44639ec..0000000000 --- a/packages/core/admin/admin/src/layouts/AppLayout/index.js +++ /dev/null @@ -1,33 +0,0 @@ -import React from 'react'; - -import { Box, Flex, SkipToContent } from '@strapi/design-system'; -import PropTypes from 'prop-types'; -import { useIntl } from 'react-intl'; -import styled from 'styled-components'; - -const FlexBox = styled(Box)` - flex: 1; -`; - -const AppLayout = ({ children, sideNav }) => { - const { formatMessage } = useIntl(); - - return ( - - - {formatMessage({ id: 'skipToContent', defaultMessage: 'Skip to content' })} - - - {sideNav} - {children} - - - ); -}; - -AppLayout.propTypes = { - children: PropTypes.node.isRequired, - sideNav: PropTypes.node.isRequired, -}; - -export default AppLayout; diff --git a/packages/core/admin/admin/src/pages/Admin/index.js b/packages/core/admin/admin/src/pages/Admin/index.js index 91b25870da..4c20154153 100644 --- a/packages/core/admin/admin/src/pages/Admin/index.js +++ b/packages/core/admin/admin/src/pages/Admin/index.js @@ -6,6 +6,7 @@ import * as React from 'react'; +import { Box, Flex } from '@strapi/design-system'; import { LoadingIndicatorPage, useStrapiApp, useTracking } from '@strapi/helper-plugin'; import { DndProvider } from 'react-dnd'; import { HTML5Backend } from 'react-dnd-html5-backend'; @@ -15,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 AppLayout from '../../layouts/AppLayout'; import createRoute from '../../utils/createRoute'; import { SET_APP_RUNTIME_STATUS } from '../App/constants'; @@ -82,33 +82,33 @@ export const Admin = () => { return ( - - } - > - }> - - - - - {routes} - - - - - - - - - - - - - + + + + + }> + + + + + {routes} + + + + + + + + + + + + + + {/* TODO: we should move the logic to determine whether the guided tour is displayed or not out of the component, to make the code-splitting more effective @@ -116,7 +116,7 @@ export const Admin = () => { {showTutorials && } - + ); };