Bug 1541631 - Part 7: Convert memoizeableAction to be AsyncValue-based for easy interop. r=jlast

Differential Revision: https://phabricator.services.mozilla.com/D42032

--HG--
extra : moz-landing-system : lando

devtools/client/debugger/src/actions/breakpoints/breakpointPositions.js

@@ -14,8 +14,6 @@ import { uniqBy, zip } from "lodash";
 import {
   getSource,
   getSourceFromId,
-  hasBreakpointPositions,
-  hasBreakpointPositionsForLine,
   getBreakpointPositionsForSource,
   getSourceActorsForSource,
 } from "../../selectors";
@@ -33,6 +31,7 @@ import {
   memoizeableAction,
   type MemoizedAction,
 } from "../../utils/memoizableAction";
+import { fulfilled } from "../../utils/async-value";
 import type { ThunkArgs } from "../../actions/types";
 
 async function mapLocations(
@@ -197,12 +196,20 @@ export const setBreakpointPositions: MemoizedAction<
   { cx: Context, sourceId: string, line?: number },
   ?BreakpointPositions
 > = memoizeableAction("setBreakpointPositions", {
-  hasValue: ({ sourceId, line }, { getState }) =>
-    isGeneratedId(sourceId) && line
-      ? hasBreakpointPositionsForLine(getState(), sourceId, line)
-      : hasBreakpointPositions(getState(), sourceId),
-  getValue: ({ sourceId, line }, { getState }) =>
-    getBreakpointPositionsForSource(getState(), sourceId),
+  getValue: ({ sourceId, line }, { getState }) => {
+    const positions = getBreakpointPositionsForSource(getState(), sourceId);
+    if (!positions) {
+      return null;
+    }
+
+    if (isGeneratedId(sourceId) && line && !positions[line]) {
+      // We always return the full position dataset, but if a given line is
+      // not available, we treat the whole set as loading.
+      return null;
+    }
+
+    return fulfilled(positions);
+  },
   createKey({ sourceId, line }, { getState }) {
     const key = generatedSourceActorKey(getState(), sourceId);
     return isGeneratedId(sourceId) && line ? `${key}-${line}` : key;

devtools/client/debugger/src/actions/sources/loadSourceText.js

@@ -19,7 +19,7 @@ import { addBreakpoint } from "../breakpoints";
 
 import { prettyPrintSource } from "./prettyPrint";
 import { setBreakableLines } from "./breakableLines";
-import { isFulfilled } from "../../utils/async-value";
+import { isFulfilled, fulfilled } from "../../utils/async-value";
 
 import { isOriginal, isPretty } from "../../utils/source";
 import {
@@ -137,17 +137,22 @@ export const loadSourceText: MemoizedAction<
   { cx: Context, source: Source },
   ?Source
 > = memoizeableAction("loadSourceText", {
-  hasValue: ({ source }, { getState }) => {
-    return (
-      !source ||
-      !!(
-        getSource(getState(), source.id) &&
-        getSourceWithContent(getState(), source.id).content
-      )
-    );
+  getValue: ({ source }, { getState }) => {
+    source = source ? getSource(getState(), source.id) : null;
+    if (!source) {
+      return null;
+    }
+
+    const { content } = getSourceWithContent(getState(), source.id);
+    if (!content || content.state === "pending") {
+      return content;
+    }
+
+    // This currently swallows source-load-failure since we return fulfilled
+    // here when content.state === "rejected". In an ideal world we should
+    // propagate that error upward.
+    return fulfilled(source);
   },
-  getValue: ({ source }, { getState }) =>
-    source ? getSource(getState(), source.id) : null,
   createKey: ({ source }, { getState }) => {
     const epoch = getSourcesEpoch(getState());
     return `${epoch}:${source.id}`;

devtools/client/debugger/src/actions/sources/symbols.js

@@ -4,7 +4,7 @@
 
 // @flow
 
-import { hasSymbols, getSymbols } from "../../selectors";
+import { getSymbols } from "../../selectors";
 
 import { PROMISE } from "../utils/middleware/promise";
 import { updateTab } from "../tabs";
@@ -14,6 +14,7 @@ import {
   memoizeableAction,
   type MemoizedAction,
 } from "../../utils/memoizableAction";
+import { fulfilled } from "../../utils/async-value";
 
 import type { Source, Context } from "../../types";
 import type { Symbols } from "../../reducers/types";
@@ -41,10 +42,18 @@ type Args = { cx: Context, source: Source };
 export const setSymbols: MemoizedAction<Args, ?Symbols> = memoizeableAction(
   "setSymbols",
   {
-    hasValue: ({ source }, { getState }) =>
-      source.isWasm || hasSymbols(getState(), source),
-    getValue: ({ source }, { getState }) =>
-      source.isWasm ? null : getSymbols(getState(), source),
+    getValue: ({ source }, { getState }) => {
+      if (source.isWasm) {
+        return fulfilled(null);
+      }
+
+      const symbols = getSymbols(getState(), source);
+      if (!symbols || symbols.loading) {
+        return null;
+      }
+
+      return fulfilled(symbols);
+    },
     createKey: ({ source }) => source.id,
     action: ({ cx, source }, thunkArgs) => doSetSymbols(cx, source, thunkArgs),
   }

devtools/client/debugger/src/reducers/sources.js

@@ -259,7 +259,7 @@ const resourceAsSourceBase = memoizeResourceShallow(
 const resourceAsSourceWithContent = memoizeResourceShallow(
   ({ content, ...source }: SourceResource): SourceWithContent => ({
     ...source,
-    content: content && content.state !== "pending" ? content : null,
+    content: asyncValue.asSettled(content),
   })
 );
 
@@ -788,12 +788,7 @@ export function getSourceContent(
   id: SourceId
 ): SettledValue<SourceContent> | null {
   const { content } = getResource(state.sources.sources, id);
-
-  if (!content || content.state === "pending") {
-    return null;
-  }
-
-  return content;
+  return asyncValue.asSettled(content);
 }
 
 export function getSelectedSourceId(state: OuterState) {

devtools/client/debugger/src/utils/async-value.js

@@ -28,6 +28,12 @@ export function rejected(value: mixed): RejectedValue {
   return { state: "rejected", value };
 }
 
+export function asSettled<T>(
+  value: AsyncValue<T> | null
+): SettledValue<T> | null {
+  return value && value.state !== "pending" ? value : null;
+}
+
 export function isPending(value: AsyncValue<mixed>): boolean %checks {
   return value.state === "pending";
 }

devtools/client/debugger/src/utils/memoizableAction.js

@@ -5,11 +5,14 @@
 // @flow
 
 import type { ThunkArgs } from "../actions/types";
+import { asSettled, type AsyncValue } from "./async-value";
 
-export type MemoizedAction<Args, Result> = Args => ThunkArgs => Promise<Result>;
+export type MemoizedAction<
+  Args,
+  Result
+> = Args => ThunkArgs => Promise<Result | null>;
 type MemoizableActionParams<Args, Result> = {
-  hasValue: (args: Args, thunkArgs: ThunkArgs) => boolean,
-  getValue: (args: Args, thunkArgs: ThunkArgs) => Result,
+  getValue: (args: Args, thunkArgs: ThunkArgs) => AsyncValue<Result> | null,
   createKey: (args: Args, thunkArgs: ThunkArgs) => string,
   action: (args: Args, thunkArgs: ThunkArgs) => Promise<mixed>,
 };
@@ -18,7 +21,6 @@ type MemoizableActionParams<Args, Result> = {
  * memoizableActon is a utility for actions that should only be performed
  * once per key. It is useful for loading sources, parsing symbols ...
  *
- * @hasValue - checks to see if the result is in the redux store
  * @getValue - gets the result from the redux store
  * @createKey - creates a key for the requests map
  * @action - kicks off the async work for the action
@@ -39,36 +41,47 @@ type MemoizableActionParams<Args, Result> = {
  */
 export function memoizeableAction<Args, Result>(
   name: string,
-  {
-    hasValue,
-    getValue,
-    createKey,
-    action,
-  }: MemoizableActionParams<Args, Result>
+  { getValue, createKey, action }: MemoizableActionParams<Args, Result>
 ): MemoizedAction<Args, Result> {
   const requests = new Map();
-  return args => async (thunkArgs: ThunkArgs) => {
-    if (hasValue(args, thunkArgs)) {
-      return getValue(args, thunkArgs);
+  return args => async thunkArgs => {
+    let result = asSettled(getValue(args, thunkArgs));
+    if (!result) {
+      const key = createKey(args, thunkArgs);
+      if (!requests.has(key)) {
+        requests.set(
+          key,
+          (async () => {
+            try {
+              await action(args, thunkArgs);
+            } catch (e) {
+              console.warn(`Action ${name} had an exception:`, e);
+            } finally {
+              requests.delete(key);
+            }
+          })()
+        );
+      }
+
+      await requests.get(key);
+
+      result = asSettled(getValue(args, thunkArgs));
+
+      if (!result) {
+        // Returning null here is not ideal. This means that the action
+        // resolved but 'getValue' didn't return a loaded value, for instance
+        // if the data the action was meant to store was deleted. In a perfect
+        // world we'd throw a ContextError here or handle cancellation somehow.
+        // Throwing will also allow us to change the return type on the action
+        // to always return a promise for the getValue AsyncValue type, but
+        // for now we have to add an additional '| null' for this case.
+        return null;
+      }
     }
 
-    const key = createKey(args, thunkArgs);
-    if (!requests.has(key)) {
-      requests.set(
-        key,
-        (async () => {
-          try {
-            await action(args, thunkArgs);
-          } catch (e) {
-            console.warn(`Action ${name} had an exception:`, e);
-          } finally {
-            requests.delete(key);
-          }
-        })()
-      );
+    if (result.state === "rejected") {
+      throw result.value;
     }
-
-    await requests.get(key);
-    return getValue(args, thunkArgs);
+    return result.value;
   };
 }