async fetching of additional datasets

src/actions/loadData.js

@@ -259,41 +259,36 @@ const fetchAndCacheNarrativeDatasets = async (dispatch, blocks, query) => {
     secondTreeDataset: false,
     secondTreeName: false
   };
-  const treeNames = _uniq(blocks.map((block) => collectDatasetFetchUrls(block.dataset)[0]));
-  // TODO:1050 A more performant fetching strategy would be fetching the dataset for the slide you land on,
-  // then fetching all the rest in the background so we can use them from the cache upon changing slides.
-  // Doing that presents the risk of a race case (if you change pages faster than a dataset can be fetched) so we are avoiding it for now.
-  // Instead we use Promise.all to ensure all the datasets are fetched before we render.
-  return Promise.all(treeNames.map((treeName) => {
-    // TODO:1050
-    // 1. allow frequencies to be loaded for a narrative dataset here
-    // 2. allow loading dataset for secondTreeName
-    return getDataset(treeName)
-      .then((res) => res.json())
-      .then((json) => {
-        jsons[treeName] = json;
-        if (treeName === startingTreeName) {
-          landingSlide.json = json;
-        }
-        return json;
-      });
-  })).then(() => {
-    // TODO:1050: this dispatch is an promise side effect which means we don't guarantee
-    // that jsons are cached before returning below. To be able to make such a guarantee,
-    // there are a number of accepted ways to do async action creator functions in redux
-    // (https://redux.js.org/faq/actions#how-can-i-represent-side-effects-such-as-ajax-calls-why-do-we-need-things-like-action-creators-thunks-and-middleware-to-do-async-behavior)
-    dispatch({
-      type: types.CACHE_JSONS,
-      jsons
-    });
-    // We don't use the returned value of Promise.all above since we
-    // have already constructed the `jsons` object in order to avoid duplicate fetching.
-    return {
-      blocks,
-      pathnameShouldBe: startingDataset,
-      landingSlide
-    };
+  const treeNames = blocks.map((block) => collectDatasetFetchUrls(block.dataset)[0]);
+
+  // TODO:1050
+  // 1. allow frequencies to be loaded for a narrative dataset here
+  // 2. allow loading dataset for secondTreeName
+
+  // We block and await for the landing dataset
+  jsons[startingTreeName] = landingSlide.json = await getDataset(startingTreeName).then(((res) => res.json());
+
+  // The other datasets are fetched asynchronously
+  for (const treeName of treeNames)
+  // With this there's no need for Set above
+    jsons[treeName] = jsons[treeName] || getDataset(treeName).then((res) => res.json());
+
+  // I don't think the below here is a real problem for any practical case (?)
+
+  // TODO:1050: this dispatch is an promise side effect which means we don't guarantee
+  // that jsons are cached before returning below. To be able to make such a guarantee,
+  // there are a number of accepted ways to do async action creator functions in redux
+  // (https://redux.js.org/faq/actions#how-can-i-represent-side-effects-such-as-ajax-calls-why-do-we-need-things-like-action-creators-thunks-and-middleware-to-do-async-behavior)
+  dispatch({
+    type: types.CACHE_JSONS,
+    jsons
   });
+
+  return {
+    blocks,
+    pathnameShouldBe: startingDataset,
+    landingSlide
+  };
 };
 
 

src/actions/navigation.js

@@ -25,27 +25,19 @@ export const chooseDisplayComponentFromURL = (url) => {
   return "datasetLoader"; // fallthrough
 };
 
-/* Try to get a JSON from the redux state where it may already have
- * been cached. If it's not there, we should fetch it. The latter part
- * has yet to be implemented since the only instance where we cache JSONs
- * at this point fetches all needed JSONs by a given narrative before rendering.
- * In the future, when we allow for more performative loading of JSONs in the backgroud,
- * we won't be able to predict their existence in the cache and so will need to fetch
- * here upon a "cache miss"
+/*
+ * All the Fetch Promises are created before first render. When trying the cache we `await`.
+ * If the Fetch is not finished, this will wait for it to end. Subsequent awaits will immeditaly return the result. 
+ * For the landing dataset, no problem either because await on a value just returns the value. 
  */
 const tryCacheThenFetch = async (mainTreeName, secondTreeName, state) => {
   if (state.jsonCache && state.jsonCache.jsons && state.jsonCache.jsons[mainTreeName] !== undefined) {
     return {
-      json: state.jsonCache.jsons[mainTreeName],
-      secondJson: state.jsonCache.jsons[secondTreeName]
+      json: await state.jsonCache.jsons[mainTreeName],
+      secondJson: await state.jsonCache.jsons[secondTreeName]
     };
   }
-  // TODO:1050 we should fetch a dataset here if it is not in the cache
-  // instead of throwing an error. As the error message suggests, the reason
-  // we throw is because our current fetching strategy should prevent a cache miss.
-  // We will want to implement a real fetch here when we move to a more performant
-  // fetching strategy (see actions/loadData.js)
-  throw new Error("This should not happen given that we naively pre-fetch all datasets for any narrative before rendering");
+  throw new Error("This should not happen given that we start fetching all datasets before rendering");
 };
 
 /* changes the state of the page and (perhaps) the dataset displayed.