From 38d13deafac6caeb0e34f86f3cf0b567baf66d4b Mon Sep 17 00:00:00 2001 From: Josh <37798644+joshuaellis@users.noreply.github.com> Date: Fri, 2 Dec 2022 17:23:00 +0000 Subject: [PATCH 1/4] fix: use a global cache for custom inputs so they can be accessed anywhere --- .../NonRepeatableComponent/index.js | 4 ++ .../RepeatableComponent/DraggedItem/index.js | 4 ++ .../hooks/useLazyComponents/index.js | 49 ++++++++++++++----- 3 files changed, 46 insertions(+), 11 deletions(-) diff --git a/packages/core/admin/admin/src/content-manager/components/NonRepeatableComponent/index.js b/packages/core/admin/admin/src/content-manager/components/NonRepeatableComponent/index.js index 721472d1ab..06c0031153 100644 --- a/packages/core/admin/admin/src/content-manager/components/NonRepeatableComponent/index.js +++ b/packages/core/admin/admin/src/content-manager/components/NonRepeatableComponent/index.js @@ -9,6 +9,7 @@ import { Stack } from '@strapi/design-system/Stack'; import { useContentTypeLayout } from '../../hooks'; import FieldComponent from '../FieldComponent'; import Inputs from '../Inputs'; +import useLazyComponents from '../../hooks/useLazyComponents'; const NonRepeatableComponent = ({ componentUid, isFromDynamicZone, isNested, name }) => { const { getComponentLayout } = useContentTypeLayout(); @@ -18,6 +19,8 @@ const NonRepeatableComponent = ({ componentUid, isFromDynamicZone, isNested, nam ); const fields = componentLayoutData.layouts.edit; + const { lazyComponentStore } = useLazyComponents(); + return ( ); diff --git a/packages/core/admin/admin/src/content-manager/components/RepeatableComponent/DraggedItem/index.js b/packages/core/admin/admin/src/content-manager/components/RepeatableComponent/DraggedItem/index.js index 41fcb02fc1..50d89e4f01 100644 --- a/packages/core/admin/admin/src/content-manager/components/RepeatableComponent/DraggedItem/index.js +++ b/packages/core/admin/admin/src/content-manager/components/RepeatableComponent/DraggedItem/index.js @@ -21,6 +21,7 @@ import Preview from './Preview'; import DraggingSibling from './DraggingSibling'; import { CustomIconButton } from './IconButtonCustoms'; import { connect, select } from './utils'; +import useLazyComponents from '../../../hooks/useLazyComponents'; const DragButton = styled.span` display: flex; @@ -177,6 +178,8 @@ const DraggedItem = ({ const accordionTitle = toString(displayedValue); const accordionHasError = hasErrors ? 'error' : undefined; + const { lazyComponentStore } = useLazyComponents(); + return ( {isDragging && } @@ -273,6 +276,7 @@ const DraggedItem = ({ // onBlur={hasErrors ? checkFormErrors : null} queryInfos={queryInfos} size={size} + customFieldInputs={lazyComponentStore} /> ); diff --git a/packages/core/admin/admin/src/content-manager/hooks/useLazyComponents/index.js b/packages/core/admin/admin/src/content-manager/hooks/useLazyComponents/index.js index 1b3ddf0462..496eefc9c4 100644 --- a/packages/core/admin/admin/src/content-manager/hooks/useLazyComponents/index.js +++ b/packages/core/admin/admin/src/content-manager/hooks/useLazyComponents/index.js @@ -1,42 +1,69 @@ import { useEffect, useState } from 'react'; import { useCustomFields } from '@strapi/helper-plugin'; +const componentStore = new Map(); + /** * @description * A hook to lazy load custom field components * @param {Array.} componentUids - The uids to look up components * @returns object */ -const useLazyComponents = (componentUids) => { +const useLazyComponents = (componentUids = []) => { const [lazyComponentStore, setLazyComponentStore] = useState({}); const [loading, setLoading] = useState(true); const customFieldsRegistry = useCustomFields(); useEffect(() => { + const setStore = (store) => { + setLazyComponentStore(store); + setLoading(false); + }; + + const populateStoreWithCache = () => { + const internalStore = {}; + + componentStore.forEach((comp, uid) => { + internalStore[uid] = comp; + }); + + setStore(internalStore); + }; + const lazyLoadComponents = async (uids, components) => { const modules = await Promise.all(components); + const internalStore = {}; + uids.forEach((uid, index) => { - if (!Object.keys(lazyComponentStore).includes(uid)) { - setLazyComponentStore({ ...lazyComponentStore, [uid]: modules[index].default }); - } + componentStore.set(uid, modules[index].default); + internalStore[uid] = modules[index].default; }); + + setStore(internalStore); }; if (componentUids.length) { - const componentPromises = componentUids.map((uid) => { + /** + * These uids are not in the component store therefore we need to get the components + */ + const newUids = componentUids.filter((uid) => !componentStore.get(uid)); + + const componentPromises = newUids.map((uid) => { const customField = customFieldsRegistry.get(uid); return customField.components.Input(); }); - lazyLoadComponents(componentUids, componentPromises); + if (componentPromises.length > 0) { + lazyLoadComponents(newUids, componentPromises); + } else { + populateStoreWithCache(); + } + } else if (loading) { + populateStoreWithCache(); } - - if (componentUids.length === Object.keys(lazyComponentStore).length) { - setLoading(false); - } - }, [componentUids, customFieldsRegistry, loading, lazyComponentStore]); + }, [componentUids, customFieldsRegistry, loading]); return { isLazyLoading: loading, lazyComponentStore }; }; From 52a4811d5d1e34e34515e69d40af4daed2754669 Mon Sep 17 00:00:00 2001 From: Josh <37798644+joshuaellis@users.noreply.github.com> Date: Fri, 2 Dec 2022 17:43:57 +0000 Subject: [PATCH 2/4] refactor: make it quicker to access cache --- .../hooks/useLazyComponents/index.js | 21 +++++-------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/packages/core/admin/admin/src/content-manager/hooks/useLazyComponents/index.js b/packages/core/admin/admin/src/content-manager/hooks/useLazyComponents/index.js index 496eefc9c4..bce83e426e 100644 --- a/packages/core/admin/admin/src/content-manager/hooks/useLazyComponents/index.js +++ b/packages/core/admin/admin/src/content-manager/hooks/useLazyComponents/index.js @@ -10,8 +10,8 @@ const componentStore = new Map(); * @returns object */ const useLazyComponents = (componentUids = []) => { - const [lazyComponentStore, setLazyComponentStore] = useState({}); - const [loading, setLoading] = useState(true); + const [lazyComponentStore, setLazyComponentStore] = useState(Object.fromEntries(componentStore)); + const [loading, setLoading] = useState(componentStore.size === 0); const customFieldsRegistry = useCustomFields(); useEffect(() => { @@ -20,16 +20,6 @@ const useLazyComponents = (componentUids = []) => { setLoading(false); }; - const populateStoreWithCache = () => { - const internalStore = {}; - - componentStore.forEach((comp, uid) => { - internalStore[uid] = comp; - }); - - setStore(internalStore); - }; - const lazyLoadComponents = async (uids, components) => { const modules = await Promise.all(components); @@ -43,7 +33,7 @@ const useLazyComponents = (componentUids = []) => { setStore(internalStore); }; - if (componentUids.length) { + if (componentUids.length && loading) { /** * These uids are not in the component store therefore we need to get the components */ @@ -58,10 +48,9 @@ const useLazyComponents = (componentUids = []) => { if (componentPromises.length > 0) { lazyLoadComponents(newUids, componentPromises); } else { - populateStoreWithCache(); + const store = Object.fromEntries(componentStore); + setStore(store); } - } else if (loading) { - populateStoreWithCache(); } }, [componentUids, customFieldsRegistry, loading]); From e66352c649f9161341b3c9d2a58e788b9a4fba5b Mon Sep 17 00:00:00 2001 From: Josh <37798644+joshuaellis@users.noreply.github.com> Date: Mon, 5 Dec 2022 08:41:57 +0000 Subject: [PATCH 3/4] test: fix test for hook & add one surrounding cache --- .../hooks/useLazyComponents/index.js | 21 ++++++-- .../tests/useLazyComponents.test.js | 49 ++++++++++++++----- 2 files changed, 55 insertions(+), 15 deletions(-) diff --git a/packages/core/admin/admin/src/content-manager/hooks/useLazyComponents/index.js b/packages/core/admin/admin/src/content-manager/hooks/useLazyComponents/index.js index bce83e426e..892188c43f 100644 --- a/packages/core/admin/admin/src/content-manager/hooks/useLazyComponents/index.js +++ b/packages/core/admin/admin/src/content-manager/hooks/useLazyComponents/index.js @@ -1,4 +1,4 @@ -import { useEffect, useState } from 'react'; +import { useCallback, useEffect, useState } from 'react'; import { useCustomFields } from '@strapi/helper-plugin'; const componentStore = new Map(); @@ -11,7 +11,13 @@ const componentStore = new Map(); */ const useLazyComponents = (componentUids = []) => { const [lazyComponentStore, setLazyComponentStore] = useState(Object.fromEntries(componentStore)); - const [loading, setLoading] = useState(componentStore.size === 0); + const [loading, setLoading] = useState(() => { + if (componentStore.size === 0 && componentUids.length > 0) { + return true; + } + + return false; + }); const customFieldsRegistry = useCustomFields(); useEffect(() => { @@ -54,7 +60,16 @@ const useLazyComponents = (componentUids = []) => { } }, [componentUids, customFieldsRegistry, loading]); - return { isLazyLoading: loading, lazyComponentStore }; + /** + * Wrap this in a callback so it can be used in + * effects to cleanup the cached store if required + */ + const cleanup = useCallback(() => { + componentStore.clear(); + setLazyComponentStore({}); + }, []); + + return { isLazyLoading: loading, lazyComponentStore, cleanup }; }; export default useLazyComponents; diff --git a/packages/core/admin/admin/src/content-manager/hooks/useLazyComponents/tests/useLazyComponents.test.js b/packages/core/admin/admin/src/content-manager/hooks/useLazyComponents/tests/useLazyComponents.test.js index e5effa9a15..db825503be 100644 --- a/packages/core/admin/admin/src/content-manager/hooks/useLazyComponents/tests/useLazyComponents.test.js +++ b/packages/core/admin/admin/src/content-manager/hooks/useLazyComponents/tests/useLazyComponents.test.js @@ -26,25 +26,50 @@ jest.mock('@strapi/helper-plugin', () => ({ })); describe('useLazyComponents', () => { + let cleanup; + + afterEach(() => { + if (typeof cleanup === 'function') { + cleanup(); + cleanup = undefined; + } + }); + it('lazy loads the components', async () => { - const { result, waitForNextUpdate } = renderHook(() => + const { result, waitFor } = renderHook(() => useLazyComponents(['plugin::test.test'])); + + cleanup = result.current.cleanup; + + expect(result.current.isLazyLoading).toEqual(true); + expect(result.current.lazyComponentStore).toEqual({}); + + await waitFor(() => expect(result.current.isLazyLoading).toEqual(false)); + + expect(result.current.lazyComponentStore['plugin::test.test']).toBeDefined(); + }); + + test('assuming the store has been initialised before hand, other hooks called should be able to access the global cache', async () => { + const { result: initialResult, waitFor } = renderHook(() => useLazyComponents(['plugin::test.test']) ); - expect(result.current).toEqual({ isLazyLoading: true, lazyComponentStore: {} }); - - await waitForNextUpdate(); - - expect(JSON.stringify(result.current)).toEqual( - JSON.stringify({ - isLazyLoading: false, - lazyComponentStore: { 'plugin::test.test': jest.fn() }, - }) + await waitFor(() => + expect(initialResult.current.lazyComponentStore['plugin::test.test']).toBeDefined() ); + + const { result: actualResult, waitFor: secondWaitFor } = renderHook(() => useLazyComponents()); + + cleanup = actualResult.current.cleanup; + + await secondWaitFor(() => expect(actualResult.current.isLazyLoading).toBe(false)); + + expect(actualResult.current.lazyComponentStore['plugin::test.test']).toBeDefined(); }); - it('handles no components to load', async () => { + + test('given there are no components to load it should not be loading and the store should be empty', async () => { const { result } = renderHook(() => useLazyComponents([])); - expect(result.current).toEqual({ isLazyLoading: false, lazyComponentStore: {} }); + expect(result.current.isLazyLoading).toEqual(false); + expect(result.current.lazyComponentStore).toEqual({}); }); }); From fd8163d37b8920cc004d231cbbbf152dacca48d5 Mon Sep 17 00:00:00 2001 From: Josh <37798644+joshuaellis@users.noreply.github.com> Date: Mon, 5 Dec 2022 09:31:25 +0000 Subject: [PATCH 4/4] chore: pr amends --- .../src/content-manager/hooks/useLazyComponents/index.js | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/packages/core/admin/admin/src/content-manager/hooks/useLazyComponents/index.js b/packages/core/admin/admin/src/content-manager/hooks/useLazyComponents/index.js index 892188c43f..59bd9f4904 100644 --- a/packages/core/admin/admin/src/content-manager/hooks/useLazyComponents/index.js +++ b/packages/core/admin/admin/src/content-manager/hooks/useLazyComponents/index.js @@ -29,14 +29,11 @@ const useLazyComponents = (componentUids = []) => { const lazyLoadComponents = async (uids, components) => { const modules = await Promise.all(components); - const internalStore = {}; - uids.forEach((uid, index) => { componentStore.set(uid, modules[index].default); - internalStore[uid] = modules[index].default; }); - setStore(internalStore); + setStore(Object.fromEntries(componentStore)); }; if (componentUids.length && loading) { @@ -53,9 +50,6 @@ const useLazyComponents = (componentUids = []) => { if (componentPromises.length > 0) { lazyLoadComponents(newUids, componentPromises); - } else { - const store = Object.fromEntries(componentStore); - setStore(store); } } }, [componentUids, customFieldsRegistry, loading]);