refactored loading states & added loading in outline (#5651)

refactored symbol actions & reducers to show proper loading flag

Author: रोहन मल्होत्रा

src/actions/ast.js

@@ -18,6 +18,7 @@ import {
   findOutOfScopeLocations,
   getFramework
 } from "../workers/parser";
+import { PROMISE } from "./utils/middleware/promise";
 
 import type { SourceId } from "../types";
 import type { ThunkArgs } from "./types";
@@ -57,8 +58,12 @@ export function setSymbols(sourceId: SourceId) {
       return;
     }
 
-    const symbols = await getSymbols(source.id);
-    dispatch({ type: "SET_SYMBOLS", source, symbols });
+    await dispatch({
+      type: "SET_SYMBOLS",
+      source,
+      [PROMISE]: getSymbols(source.id)
+    });
+
     dispatch(setEmptyLines(sourceId));
     dispatch(setSourceMetaData(sourceId));
   };

src/actions/preview.js

@@ -125,7 +125,7 @@ export function updatePreview(target: HTMLElement, editor: any) {
     const symbols = getSymbols(getState(), source.toJS());
 
     let match;
-    if (!symbols || symbols.identifiers.length > 0) {
+    if (!symbols || symbols.loading) {
       match = findBestMatchExpression(symbols, tokenPos, tokenText);
     } else {
       match = getExpressionFromCoords(editor.codeMirror, tokenPos);

src/actions/tests/ast.spec.js

@@ -16,7 +16,8 @@ const {
   getEmptyLines,
   getOutOfScopeLocations,
   getSourceMetaData,
-  getInScopeLines
+  getInScopeLines,
+  isSymbolsLoading
 } = selectors;
 
 import I from "immutable";
@@ -114,32 +115,29 @@ describe("ast", () => {
         await dispatch(actions.newSource(base));
         await dispatch(actions.loadSourceText(I.Map({ id: "base.js" })));
         await dispatch(actions.setSymbols("base.js"));
-        await waitForState(
-          store,
-          state => getSymbols(state, base).functions.length > 0
-        );
+        await waitForState(store, state => !isSymbolsLoading(state, base));
 
         const baseSymbols = getSymbols(getState(), base);
         expect(baseSymbols).toMatchSnapshot();
       });
     });
 
     describe("when the source is not loaded", () => {
-      it("should return an empty set", async () => {
+      it("should return null", async () => {
         const { getState, dispatch } = createStore(threadClient);
         const base = makeSource("base.js");
         await dispatch(actions.newSource(base));
 
         const baseSymbols = getSymbols(getState(), base);
-        expect(baseSymbols).toEqual({ variables: [], functions: [] });
+        expect(baseSymbols).toEqual(null);
       });
     });
 
     describe("when there is no source", () => {
-      it("should return an empty set", async () => {
+      it("should return null", async () => {
         const { getState } = createStore(threadClient);
         const baseSymbols = getSymbols(getState());
-        expect(baseSymbols).toEqual({ variables: [], functions: [] });
+        expect(baseSymbols).toEqual(null);
       });
     });
   });

src/actions/types.js

@@ -315,7 +315,7 @@ type ASTAction =
   | {
       type: "SET_SYMBOLS",
       source: Source,
-      symbols: SymbolDeclaration[]
+      value: SymbolDeclaration[]
     }
   | {
       type: "SET_EMPTY_LINES",

src/components/PrimaryPanes/Outline.js

@@ -47,6 +47,12 @@ export class Outline extends Component<Props> {
     return <div className="outline-pane-info">{placeholderMessage}</div>;
   }
 
+  renderLoading() {
+    return (
+      <div className="outline-pane-info">{L10N.getStr("loadingText")}</div>
+    );
+  }
+
   renderFunction(func: SymbolDeclaration) {
     const { name, location, parameterNames } = func;
 
@@ -132,11 +138,12 @@ export class Outline extends Component<Props> {
 
   render() {
     const { symbols } = this.props;
-
+    if (!symbols || symbols.loading) {
+      return this.renderLoading();
+    }
     const symbolsToDisplay = symbols.functions.filter(
       func => func.name != "anonymous"
     );
-
     return (
       <div className="outline">
         {symbolsToDisplay.length > 0

src/reducers/ast.js

@@ -20,7 +20,8 @@ import type { Action } from "../actions/types";
 import type { Record } from "../utils/makeRecord";
 
 type EmptyLinesType = number[];
-export type SymbolsMap = Map<string, SymbolDeclarations>;
+export type Symbols = SymbolDeclarations | { loading: true };
+export type SymbolsMap = Map<string, Symbols>;
 export type EmptyLinesMap = Map<string, EmptyLinesType>;
 
 export type SourceMetaDataType = {
@@ -70,10 +71,12 @@ function update(
 ): Record<ASTState> {
   switch (action.type) {
     case "SET_SYMBOLS": {
-      const { source, symbols } = action;
-      return state.setIn(["symbols", source.id], symbols);
+      const { source } = action;
+      if (action.status === "start") {
+        return state.setIn(["symbols", source.id], { loading: true });
+      }
+      return state.setIn(["symbols", source.id], action.value);
     }
-
     case "SET_EMPTY_LINES": {
       const { source, emptyLines } = action;
       return state.setIn(["emptyLines", source.id], emptyLines);
@@ -131,25 +134,34 @@ function update(
 // https://github.com/devtools-html/debugger.html/blob/master/src/reducers/sources.js#L179-L185
 type OuterState = { ast: Record<ASTState> };
 
-const emptySymbols = { variables: [], functions: [] };
 export function getSymbols(
   state: OuterState,
   source: Source
-): SymbolDeclarations {
+): ?SymbolDeclarations {
   if (!source) {
-    return emptySymbols;
+    return null;
   }
 
-  const symbols = state.ast.getIn(["symbols", source.id]);
-  return symbols || emptySymbols;
+  return state.ast.getIn(["symbols", source.id]) || null;
 }
 
 export function hasSymbols(state: OuterState, source: Source): boolean {
-  if (!source) {
+  const symbols = getSymbols(state, source);
+
+  if (!symbols) {
+    return false;
+  }
+
+  return !symbols.loading;
+}
+
+export function isSymbolsLoading(state: OuterState, source: Source): boolean {
+  const symbols = getSymbols(state, source);
+  if (!symbols) {
     return false;
   }
 
-  return !!state.ast.getIn(["symbols", source.id]);
+  return !!symbols.loading;
 }
 
 export function isEmptyLineInSource(

src/utils/breakpoint/astBreakpointLocation.js

@@ -52,7 +52,7 @@ export function getASTLocation(
   symbols: SymbolDeclarations,
   location: Location
 ): ASTLocation {
-  if (source.isWasm) {
+  if (source.isWasm || !symbols || symbols.loading) {
     return { name: undefined, offset: location };
   }