Fix permanent loading state (#5645)

Author: jasonLaster

src/actions/sources/loadSourceText.js

@@ -8,7 +8,7 @@ import { isOriginalId } from "devtools-source-map";
 import { PROMISE } from "../utils/middleware/promise";
 import { getSource, getGeneratedSource } from "../../selectors";
 import * as parser from "../../workers/parser";
-import { isLoading, isLoaded } from "../../utils/source";
+import { isLoaded } from "../../utils/source";
 
 import defer from "../../utils/defer";
 import type { ThunkArgs } from "../types";
@@ -41,20 +41,21 @@ async function loadSource(source: SourceRecord, { sourceMaps, client }) {
  */
 export function loadSourceText(source: SourceRecord) {
   return async ({ dispatch, getState, client, sourceMaps }: ThunkArgs) => {
-    const telemetryStart = performance.now();
-    const deferred = defer();
+    const id = source.get("id");
 
     // Fetch the source text only once.
-    if (isLoaded(source)) {
-      return Promise.resolve(source);
+    if (requests.has(id)) {
+      return requests.get(id);
     }
 
-    const id = source.get("id");
-    if (isLoading(source) || requests.has(id)) {
-      return requests.get(id);
+    if (isLoaded(source)) {
+      return Promise.resolve();
     }
 
+    const telemetryStart = performance.now();
+    const deferred = defer();
     requests.set(id, deferred.promise);
+
     try {
       await dispatch({
         type: "LOAD_SOURCE_TEXT",

src/actions/sources/newSources.js

@@ -22,7 +22,7 @@ import {
   getPendingBreakpointsForSource
 } from "../../selectors";
 
-import type { Source } from "../../types";
+import type { Source, SourceId } from "../../types";
 import type { ThunkArgs } from "../types";
 
 function createOriginalSource(
@@ -44,15 +44,20 @@ function createOriginalSource(
  * @memberof actions/sources
  * @static
  */
-export function loadSourceMap(generatedSource: Source) {
+function loadSourceMap(generatedSourceId: SourceId) {
   return async function({ dispatch, getState, sourceMaps }: ThunkArgs) {
-    let urls;
+    const generatedSource = getSource(getState(), generatedSourceId).toJS();
+    if (!generatedSource.sourceMapURL) {
+      return;
+    }
+
+    let urls = null;
     try {
       urls = await sourceMaps.getOriginalURLs(generatedSource);
     } catch (e) {
       console.error(e);
-      urls = null;
     }
+
     if (!urls) {
       // If this source doesn't have a sourcemap, enable it for pretty printing
       dispatch({
@@ -66,10 +71,6 @@ export function loadSourceMap(generatedSource: Source) {
       createOriginalSource(url, generatedSource, sourceMaps)
     );
 
-    // TODO: check if this line is really needed, it introduces
-    // a lot of lag to the application.
-    const generatedSourceRecord = getSource(getState(), generatedSource.id);
-    await dispatch(loadSourceText(generatedSourceRecord));
     dispatch(newSources(originalSources));
   };
 }
@@ -171,7 +172,7 @@ export function newSources(sources: Source[]) {
     }
 
     await Promise.all(
-      filteredSources.map(source => dispatch(loadSourceMap(source)))
+      filteredSources.map(source => dispatch(loadSourceMap(source.id)))
     );
     // We would like to restore the blackboxed state
     // after loading all states to make sure the correctness.

src/actions/sources/tests/newSources.spec.js

@@ -40,4 +40,31 @@ describe("sources - new sources", () => {
     await dispatch(actions.newSource(baseSource));
     expect(getSelectedSource(getState()).get("url")).toBe(baseSource.url);
   });
+
+  it("should add original sources", async () => {
+    const { dispatch, getState } = createStore(
+      threadClient,
+      {},
+      {
+        getOriginalURLs: () => Promise.resolve(["magic.js"]),
+        generatedToOriginalId: (a, b) => `${a}/${b}`
+      }
+    );
+
+    await dispatch(
+      actions.newSource(makeSource("base.js", { sourceMapURL: "base.js.map" }))
+    );
+
+    const magic = getSource(getState(), "base.js/magic.js");
+    expect(magic.get("url")).toEqual("magic.js");
+  });
+
+  // eslint-disable-next-line
+  it("should no attempt to fetch original sources if it's missing a source map url", async () => {
+    const getOriginalURLs = jest.fn();
+    const { dispatch } = createStore(threadClient, {}, { getOriginalURLs });
+
+    await dispatch(actions.newSource(makeSource("base.js")));
+    expect(getOriginalURLs).not.toHaveBeenCalled();
+  });
 });

src/reducers/sources.js

@@ -170,6 +170,7 @@ function getTextPropsFromAction(action: any) {
   } else if (action.status === "error") {
     return { id: sourceId, error: action.error, loadedState: "loaded" };
   }
+
   return {
     text: value.text,
     id: sourceId,
@@ -191,12 +192,14 @@ function updateSource(state: Record<SourcesState>, source: Source | Object) {
   if (!source.id) {
     return state;
   }
+
   const existingSource = state.getIn(["sources", source.id]);
 
   if (existingSource) {
     const updatedSource = existingSource.merge(source);
     return state.setIn(["sources", source.id], updatedSource);
   }
+
   return state.setIn(["sources", source.id], new SourceRecordClass(source));
 }