From 385b75a86178741fe7fc2ac321c8c94c2da34959 Mon Sep 17 00:00:00 2001 From: Will Harney Date: Thu, 19 Jan 2023 10:45:40 -0500 Subject: [PATCH 01/10] Remove unnecessary catch clause. --- .../app/contexts/index.js | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/packages/template-retail-react-app/app/contexts/index.js b/packages/template-retail-react-app/app/contexts/index.js index 9336e1de9a..2aa9a1be34 100644 --- a/packages/template-retail-react-app/app/contexts/index.js +++ b/packages/template-retail-react-app/app/contexts/index.js @@ -88,17 +88,13 @@ export const CategoriesProvider = ({treeRoot = {}, children, locale}) => { return error } }) - ) - .then((data) => { - const newTree = { - ...root, - [itemsKey]: data - } - setRoot(newTree) - }) - .catch((err) => { - throw err - }) + ).then((data) => { + const newTree = { + ...root, + [itemsKey]: data + } + setRoot(newTree) + }) }, []) return ( From 7d9624ba0f2c4160aafd7ce946590ae16502e662 Mon Sep 17 00:00:00 2001 From: Will Harney Date: Thu, 19 Jan 2023 10:46:18 -0500 Subject: [PATCH 02/10] Update copyright. --- packages/template-retail-react-app/app/contexts/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/template-retail-react-app/app/contexts/index.js b/packages/template-retail-react-app/app/contexts/index.js index 2aa9a1be34..3f638a91b8 100644 --- a/packages/template-retail-react-app/app/contexts/index.js +++ b/packages/template-retail-react-app/app/contexts/index.js @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, salesforce.com, inc. + * Copyright (c) 2023, Salesforce, Inc. * All rights reserved. * SPDX-License-Identifier: BSD-3-Clause * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/BSD-3-Clause From 8f65c129ce988ea33a2f3316b89446d7b5b8b80f Mon Sep 17 00:00:00 2001 From: Will Harney Date: Thu, 19 Jan 2023 10:47:01 -0500 Subject: [PATCH 03/10] Don't return error when category is expected. --- .../app/contexts/index.js | 27 ++++++++----------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/packages/template-retail-react-app/app/contexts/index.js b/packages/template-retail-react-app/app/contexts/index.js index 3f638a91b8..e1682d35fb 100644 --- a/packages/template-retail-react-app/app/contexts/index.js +++ b/packages/template-retail-react-app/app/contexts/index.js @@ -71,22 +71,17 @@ export const CategoriesProvider = ({treeRoot = {}, children, locale}) => { root?.[itemsKey]?.map(async (cat) => { // check localstorage first for this data to help remediate O(n) server // load burden where n = top level categories - let res - try { - res = await fetchCategoryNode(cat?.id, 2) - // store fetched data in local storage for faster access / reduced server load - res.loaded = true - window?.localStorage?.setItem( - `${LOCAL_STORAGE_PREFIX}${cat?.id}-${locale}`, - JSON.stringify({ - ...res, - fetchTime: Date.now() - }) - ) - return res - } catch (error) { - return error - } + const res = await fetchCategoryNode(cat?.id, 2) + // store fetched data in local storage for faster access / reduced server load + res.loaded = true + window?.localStorage?.setItem( + `${LOCAL_STORAGE_PREFIX}${cat?.id}-${locale}`, + JSON.stringify({ + ...res, + fetchTime: Date.now() + }) + ) + return res }) ).then((data) => { const newTree = { From 85f17adacaa35f604cece2d3db4b12c843436c00 Mon Sep 17 00:00:00 2001 From: Will Harney Date: Thu, 19 Jan 2023 10:53:46 -0500 Subject: [PATCH 04/10] Fix bug preventing cache invalidation. --- packages/template-retail-react-app/app/contexts/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/template-retail-react-app/app/contexts/index.js b/packages/template-retail-react-app/app/contexts/index.js index e1682d35fb..e59dae76e6 100644 --- a/packages/template-retail-react-app/app/contexts/index.js +++ b/packages/template-retail-react-app/app/contexts/index.js @@ -52,7 +52,7 @@ export const CategoriesProvider = ({treeRoot = {}, children, locale}) => { const storageItem = JSON.parse( window.localStorage.getItem(`${LOCAL_STORAGE_PREFIX}${id}-${locale}`) ) - if (storageItem || Date.now() < storageItem?.fetchTime + CAT_MENU_STALE_TIME) { + if (storageItem && Date.now() < storageItem?.fetchTime + CAT_MENU_STALE_TIME) { return storageItem } const res = await api.shopperProducts.getCategory({ From cc12695338f9c9dd1d2c99534257de4abdb51ae8 Mon Sep 17 00:00:00 2001 From: Will Harney Date: Thu, 19 Jan 2023 11:06:10 -0500 Subject: [PATCH 05/10] Set fetchTime when data is fetched, rather than every page load. --- .../app/contexts/index.js | 48 +++++++++---------- 1 file changed, 22 insertions(+), 26 deletions(-) diff --git a/packages/template-retail-react-app/app/contexts/index.js b/packages/template-retail-react-app/app/contexts/index.js index e59dae76e6..ad9cdb1d25 100644 --- a/packages/template-retail-react-app/app/contexts/index.js +++ b/packages/template-retail-react-app/app/contexts/index.js @@ -48,48 +48,44 @@ export const CategoriesProvider = ({treeRoot = {}, children, locale}) => { }) const fetchCategoryNode = async (id, levels = 1) => { + const storageKey = `${LOCAL_STORAGE_PREFIX}${id}-${locale}` + const storageItem = JSON.parse(window.localStorage.getItem(storageKey)) + // return early if there's one that is less than stale time - const storageItem = JSON.parse( - window.localStorage.getItem(`${LOCAL_STORAGE_PREFIX}${id}-${locale}`) - ) - if (storageItem && Date.now() < storageItem?.fetchTime + CAT_MENU_STALE_TIME) { + if (storageItem && Date.now() < storageItem.fetchTime + CAT_MENU_STALE_TIME) { return storageItem } + // get and store fresh data for faster access / reduced server load + // TODO: Convert to useCategory hook when hooks are ready const res = await api.shopperProducts.getCategory({ parameters: { id, levels } }) + res.loaded = true + window.localStorage.setItem( + storageKey, + JSON.stringify({ + ...res, + fetchTime: Date.now() + }) + ) return res } useEffect(() => { // Server side, we only fetch level 0 categories, for performance, here // we request the remaining two levels of category depth - Promise.all( - root?.[itemsKey]?.map(async (cat) => { - // check localstorage first for this data to help remediate O(n) server - // load burden where n = top level categories - const res = await fetchCategoryNode(cat?.id, 2) - // store fetched data in local storage for faster access / reduced server load - res.loaded = true - window?.localStorage?.setItem( - `${LOCAL_STORAGE_PREFIX}${cat?.id}-${locale}`, - JSON.stringify({ - ...res, - fetchTime: Date.now() - }) - ) - return res - }) - ).then((data) => { - const newTree = { - ...root, - [itemsKey]: data + Promise.all(root?.[itemsKey]?.map(async (cat) => await fetchCategoryNode(cat?.id, 2))).then( + (data) => { + const newTree = { + ...root, + [itemsKey]: data + } + setRoot(newTree) } - setRoot(newTree) - }) + ) }, []) return ( From 1d02fc76fc7daaa09a518364a9914c699c8c7cf5 Mon Sep 17 00:00:00 2001 From: Will Harney Date: Thu, 19 Jan 2023 11:07:59 -0500 Subject: [PATCH 06/10] Update comment. --- packages/template-retail-react-app/app/contexts/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/template-retail-react-app/app/contexts/index.js b/packages/template-retail-react-app/app/contexts/index.js index ad9cdb1d25..9da8c96c21 100644 --- a/packages/template-retail-react-app/app/contexts/index.js +++ b/packages/template-retail-react-app/app/contexts/index.js @@ -40,7 +40,7 @@ export const CategoriesProvider = ({treeRoot = {}, children, locale}) => { const api = useCommerceAPI() const [root, setRoot] = useState({ - // map over the server-provided cat + // Clone the server-provided category ...treeRoot[DEFAULT_ROOT_CATEGORY], [itemsKey]: treeRoot[DEFAULT_ROOT_CATEGORY]?.[itemsKey]?.map((item) => ({ ...item From e837de668f2e5acd553e21fef09f79d812586c9e Mon Sep 17 00:00:00 2001 From: Will Harney Date: Thu, 19 Jan 2023 11:10:40 -0500 Subject: [PATCH 07/10] Convert promise chain to async/await for readability. --- .../app/contexts/index.js | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/packages/template-retail-react-app/app/contexts/index.js b/packages/template-retail-react-app/app/contexts/index.js index 9da8c96c21..046b62b8fd 100644 --- a/packages/template-retail-react-app/app/contexts/index.js +++ b/packages/template-retail-react-app/app/contexts/index.js @@ -74,18 +74,17 @@ export const CategoriesProvider = ({treeRoot = {}, children, locale}) => { return res } - useEffect(() => { + useEffect(async () => { // Server side, we only fetch level 0 categories, for performance, here // we request the remaining two levels of category depth - Promise.all(root?.[itemsKey]?.map(async (cat) => await fetchCategoryNode(cat?.id, 2))).then( - (data) => { - const newTree = { - ...root, - [itemsKey]: data - } - setRoot(newTree) - } - ) + const promises = root?.[itemsKey]?.map(async (cat) => await fetchCategoryNode(cat?.id, 2)) + // TODO: Error handling when fetching data fails. + // Possibly switch to .allSettled to show partial data? + const data = await Promise.all(promises) + setRoot({ + ...root, + [itemsKey]: data + }) }, []) return ( From 5021816ca08516640b2af54c9d17abce28f7ba19 Mon Sep 17 00:00:00 2001 From: Will Harney Date: Thu, 19 Jan 2023 11:40:28 -0500 Subject: [PATCH 08/10] Guard against missing category items. --- .../template-retail-react-app/app/contexts/index.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/template-retail-react-app/app/contexts/index.js b/packages/template-retail-react-app/app/contexts/index.js index 046b62b8fd..8134b3b844 100644 --- a/packages/template-retail-react-app/app/contexts/index.js +++ b/packages/template-retail-react-app/app/contexts/index.js @@ -80,11 +80,13 @@ export const CategoriesProvider = ({treeRoot = {}, children, locale}) => { const promises = root?.[itemsKey]?.map(async (cat) => await fetchCategoryNode(cat?.id, 2)) // TODO: Error handling when fetching data fails. // Possibly switch to .allSettled to show partial data? - const data = await Promise.all(promises) - setRoot({ - ...root, - [itemsKey]: data - }) + if (promises) { + const data = await Promise.all(promises) + setRoot({ + ...root, + [itemsKey]: data + }) + } }, []) return ( From f78ba7ba6bbd6cf29b1d65c46acbaff8a86f0ba9 Mon Sep 17 00:00:00 2001 From: Will Harney Date: Thu, 19 Jan 2023 11:45:04 -0500 Subject: [PATCH 09/10] Move comment inside code block. --- packages/template-retail-react-app/app/contexts/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/template-retail-react-app/app/contexts/index.js b/packages/template-retail-react-app/app/contexts/index.js index 8134b3b844..0611258cba 100644 --- a/packages/template-retail-react-app/app/contexts/index.js +++ b/packages/template-retail-react-app/app/contexts/index.js @@ -78,9 +78,9 @@ export const CategoriesProvider = ({treeRoot = {}, children, locale}) => { // Server side, we only fetch level 0 categories, for performance, here // we request the remaining two levels of category depth const promises = root?.[itemsKey]?.map(async (cat) => await fetchCategoryNode(cat?.id, 2)) - // TODO: Error handling when fetching data fails. - // Possibly switch to .allSettled to show partial data? if (promises) { + // TODO: Error handling when fetching data fails. + // Possibly switch to .allSettled to show partial data? const data = await Promise.all(promises) setRoot({ ...root, From cde7a5e8fad82734f318e3a3b9c3a319ce51ca2a Mon Sep 17 00:00:00 2001 From: Will Harney Date: Thu, 19 Jan 2023 12:21:20 -0500 Subject: [PATCH 10/10] Change useEffect back to promise chain, not async/await. useEffect expects the callback to return nothing or a function; returning a promise could break things. --- .../template-retail-react-app/app/contexts/index.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/template-retail-react-app/app/contexts/index.js b/packages/template-retail-react-app/app/contexts/index.js index 0611258cba..dda5a5d9e0 100644 --- a/packages/template-retail-react-app/app/contexts/index.js +++ b/packages/template-retail-react-app/app/contexts/index.js @@ -74,17 +74,18 @@ export const CategoriesProvider = ({treeRoot = {}, children, locale}) => { return res } - useEffect(async () => { + useEffect(() => { // Server side, we only fetch level 0 categories, for performance, here // we request the remaining two levels of category depth const promises = root?.[itemsKey]?.map(async (cat) => await fetchCategoryNode(cat?.id, 2)) if (promises) { // TODO: Error handling when fetching data fails. // Possibly switch to .allSettled to show partial data? - const data = await Promise.all(promises) - setRoot({ - ...root, - [itemsKey]: data + Promise.all(promises).then((data) => { + setRoot({ + ...root, + [itemsKey]: data + }) }) } }, [])