[Custom fields] Create hook to lazy load custom field components

[markkaylor]

What does it do

• Removes lazy loading custom field components via React.lazy and React.Suspense
• Creates a useLazyComponents hook that handles the lazy loading of custom field components
  • NOTE: This could be modified to handle other components in the future, like the ones registered on the Fields API
• Lazy loads the components at a higher level in the EditView page vs the Inputs component and returns the LoadingPageIndicator until all components have loaded

Why is it needed

• Custom fields currently cause a bug with InputJSON where the CSS for the json input is not loaded and the formatting doesn’t work properly (mainly the gutters don’t appear)
• React.lazy is not intended to be used inside a component, it is designed to be used outside the component with other imports: https://reactjs.org/docs/code-splitting.html, https://beta.reactjs.org/apis/react/lazy
• We were wrapping React.lazy in useMemo to prevent a re-render. I found similar workarounds here, but I think this is ill advised since the useMemo React docs state:

You may rely on useMemo as a performance optimization, not as a semantic guarantee. In the future, React may choose to “forget” some previously memoized values and recalculate them on next render, e.g. to free memory for offscreen components. Write your code so that it still works without useMemo — and then add it to optimize performance.

• Admittedly I do not fully understand the bug and my fix is based on the assumption that React.lazy + useMemo is not a reliable way to lazy load the components since both are not being used as React intended.
• I found two things that fixed the issue:
  • Moving the React.Suspense to be the direct parent of the lazy loaded component (no idea why that worked)
  • Replacing React.lazy / React.Suspense by loading the component asynchronously

How to test it

• Go to any content-type with a custom field and an a JSON field. Both should work properly. You can also try changing the network speed with dev tools throttling
  • NOTE: Chrome fast 3g does not fully load the JSON input component with or without a custom field present. This problem does not exist in firefox. I do not believe this is related to custom fields.
• Try installing and using multiple custom fields with the JSON input (you will have to yarn build && yarn develop instead of using the admin on 4000)

Related issue(s)/PR(s)

fixes #14489

[markkaylor]

ℹ️ Instead of using this just for custom fields, in the future we could replace this with the fields API if we ensure all fields added are dynamic imports and we add custom fields to the Fields object when the app starts.

[codecov]

Codecov Report

Base: 59.68% // Head: 59.73% // Increases project coverage by +0.05% 🎉

Coverage data is based on head (0343495) compared to base (94cd474).
Patch coverage: 77.58% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #14873      +/-   ##
==========================================
+ Coverage   59.68%   59.73%   +0.05%     
==========================================
  Files        1342     1345       +3     
  Lines       32629    32664      +35     
  Branches     6224     6227       +3     
==========================================
+ Hits        19473    19513      +40     
+ Misses      11293    11289       -4     
+ Partials     1863     1862       -1     

Flag	Coverage Δ
back	49.70% <ø> (ø)
front	64.30% <77.58%> (+0.07%)	⬆️
unit_back	49.70% <ø> (ø)
unit_front	64.30% <77.58%> (+0.07%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
.../admin/src/content-manager/pages/EditView/index.js	44.00% <23.07%> (-1.08%)	⬇️
...in/src/content-manager/pages/EditView/selectors.js	33.33% <33.33%> (-6.67%)	⬇️
...ctionTypeRecursivePath/components/ErrorFallback.js	83.33% <83.33%> (ø)
...min/src/content-manager/components/Inputs/index.js	16.66% <100.00%> (+1.21%)	⬆️
...c/content-manager/hooks/useLazyComponents/index.js	100.00% <100.00%> (ø)
...manager/pages/CollectionTypeRecursivePath/index.js	71.73% <100.00%> (+0.62%)	⬆️
...rc/content-manager/pages/EditView/GridRow/index.js	64.28% <100.00%> (+2.74%)	⬆️
...ges/EditView/utils/getCustomFieldUidsFromLayout.js	100.00% <100.00%> (ø)
.../src/content-manager/pages/EditView/utils/index.js	100.00% <100.00%> (ø)
...content-manager/components/InputUID/useDebounce.js	100.00% <0.00%> (+10.00%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[markkaylor]

ℹ️ Two independent arrays that then become key value pairs on an object feels a bit strange but I didn't see another way to make it work with Promise.all. I think it is reliable since Promise.all returns them in the same order, but open to other suggestions.

[remidej]

This is not a full review as I need to look more into it, but I wanted to share my first thoughts.

Also thanks for the detailed PR notes, it's super useful especially with the resources you link to

[remidej]

Thanks for adding the tests 👌

I had the chance to test it more thoroughly and this seems solid to me. My only suggestion would be to add an ErrorBoundary component where the Suspense used to be, because currently if one of the dynamic imports fails the app crashes entirely

[markkaylor]

@soupette I will apply Rémi's feedback but we were also wondering if you might have the time to give this a review?

[markkaylor]

Not sure why I have this diff for github workflows 🤷. I'm assuming husky made these changes when I committed after merging and resolving conflicts but not sure how they made it on main...

[remidej]

Good idea to use reselect here 👍