From 758fb85d88e808542eeac1d580f7fd375d880ad8 Mon Sep 17 00:00:00 2001
From: Jim O'Donnell <james.odonnell@dtc.ox.ac.uk>
Date: Mon, 14 Oct 2024 15:04:20 +0100
Subject: [PATCH] refactor(frontend): refactor data load triggers

Remove some of the complexity from the data load triggers for the data map, without breaking the Adaptions map. These triggers rerender the data map whenever we make a change to the adaptation options sidebar panel.
---
 frontend/src/lib/data-map/DataMap.tsx         | 21 ++++--------
 .../src/lib/data-map/use-data-load-trigger.ts | 33 ++++++++++---------
 2 files changed, 23 insertions(+), 31 deletions(-)

diff --git a/frontend/src/lib/data-map/DataMap.tsx b/frontend/src/lib/data-map/DataMap.tsx
index f5ccf254..4a860166 100644
--- a/frontend/src/lib/data-map/DataMap.tsx
+++ b/frontend/src/lib/data-map/DataMap.tsx
@@ -1,9 +1,8 @@
 import type { MapboxOverlay } from '@deck.gl/mapbox/typed';
 import { useMap } from 'react-map-gl/maplibre';
-import { FC, useMemo, useRef } from 'react';
+import { FC, useRef } from 'react';
 
 import { useInteractions } from 'lib/state/interactions/use-interactions';
-import { useTriggerMemo } from 'lib/hooks/use-trigger-memo';
 import { useDataLoadTrigger } from 'lib/data-map/use-data-load-trigger';
 import { InteractionGroupConfig } from 'lib/data-map/types';
 import { DeckGLOverlay } from 'lib/map/DeckGLOverlay';
@@ -40,13 +39,9 @@ function buildLayers(
 }
 
 function useTrigger(viewLayers: ViewLayer[]) {
-  const dataLoaders = useMemo(
-    () =>
-      viewLayers
-        .map((vl) => vl.dataAccessFn?.(vl.styleParams?.colorMap?.fieldSpec)?.dataLoader)
-        .filter(Boolean),
-    [viewLayers],
-  );
+  const dataLoaders = viewLayers
+    .map((vl) => vl.dataAccessFn?.(vl.styleParams?.colorMap?.fieldSpec)?.dataLoader)
+    .filter(Boolean);
 
   return useDataLoadTrigger(dataLoaders);
 }
@@ -61,7 +56,7 @@ export const DataMap: FC<{
   const { current: map } = useMap();
   const zoom = map.getMap().getZoom();
 
-  const dataLoadTrigger = useTrigger(viewLayers);
+  useTrigger(viewLayers);
 
   const { onHover, onClick, layerFilter, pickingRadius } = useInteractions(
     viewLayers,
@@ -69,11 +64,7 @@ export const DataMap: FC<{
     interactionGroups,
   );
 
-  const layers = useTriggerMemo(
-    () => buildLayers(viewLayers, viewLayersParams, zoom, firstLabelId),
-    [viewLayers, viewLayersParams, zoom, firstLabelId],
-    dataLoadTrigger,
-  );
+  const layers = buildLayers(viewLayers, viewLayersParams, zoom, firstLabelId);
 
   return (
     <DeckGLOverlay
diff --git a/frontend/src/lib/data-map/use-data-load-trigger.ts b/frontend/src/lib/data-map/use-data-load-trigger.ts
index 1ea34c50..b9f00329 100644
--- a/frontend/src/lib/data-map/use-data-load-trigger.ts
+++ b/frontend/src/lib/data-map/use-data-load-trigger.ts
@@ -1,10 +1,10 @@
 import difference from 'lodash/difference';
-import { useEffect } from 'react';
+import { useEffect, useState } from 'react';
 
 import { DataLoader } from '../data-loader/data-loader';
 import { usePrevious } from '../hooks/use-previous';
-import { useTrackingRef } from '../hooks/use-tracking-ref';
-import { useTrigger } from '../hooks/use-trigger';
+
+const DEFAULT_LOADERS: DataLoader<any>[] = [];
 
 /**
  * Based on a list of data loaders extracted from view layers,
@@ -12,35 +12,36 @@ import { useTrigger } from '../hooks/use-trigger';
  *
  */
 export function useDataLoadTrigger(dataLoaders: DataLoader<any>[]) {
-  const [dataLoadTrigger, doTriggerDataUpdate] = useTrigger();
-
-  const previousLoaders = usePrevious(dataLoaders);
+  const [trigger, setTrigger] = useState(0);
+  const previousLoaders = usePrevious(dataLoaders) ?? DEFAULT_LOADERS;
+  const removedLoaders = difference(previousLoaders, dataLoaders);
+  const addedLoaders = difference(dataLoaders, previousLoaders);
 
   useEffect(() => {
+    function incrementTrigger() {
+      setTrigger((trigger) => trigger + 1);
+    }
     // destroy removed data loaders to free up memory
-    const removedLoaders = difference(previousLoaders ?? [], dataLoaders);
     removedLoaders.forEach((dl) => dl.destroy());
 
-    // subscribe to new data loaders to get notified when data is loaded
-    const addedLoaders = difference(dataLoaders, previousLoaders ?? []);
-    addedLoaders.forEach((dl) => dl.subscribe(doTriggerDataUpdate));
+    // subscribe to new data loaders to trigger an update to the data map when data is loaded
+    addedLoaders.forEach((dl) => dl.subscribe(incrementTrigger));
 
     // if there was a change in data loaders, trigger an update to the data map
     if (addedLoaders.length > 0 || removedLoaders.length > 0) {
-      doTriggerDataUpdate();
+      incrementTrigger();
     }
-  }, [dataLoaders, previousLoaders, doTriggerDataUpdate]);
+  }, [addedLoaders, removedLoaders]);
 
   /* store current value of dataLoaders so that we can clean up data on component unmount
    * this is necessary because we don't want to keep the data loaders around after the component is unmounted
    */
-  const currentLoadersRef = useTrackingRef(dataLoaders);
   useEffect(() => {
     return () => {
       // eslint-disable-next-line react-hooks/exhaustive-deps
-      currentLoadersRef.current?.forEach((dl) => dl.destroy());
+      previousLoaders?.forEach((dl) => dl.destroy());
     };
-  }, [currentLoadersRef]);
+  }, []);
 
-  return dataLoadTrigger;
+  return trigger;
 }