Bug 1536196 - loadSourceText should probably be responsible for pretty-printing files. r=loganfsmyth

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

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

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

@@ -5,11 +5,17 @@
 // @flow
 
 import { PROMISE } from "../utils/middleware/promise";
-import { getSource, getSourcesEpoch } from "../../selectors";
+import {
+  getSource,
+  getGeneratedSource,
+  getSourcesEpoch
+} from "../../selectors";
 import { setBreakpointPositions } from "../breakpoints";
 
+import { prettyPrintSource } from "./prettyPrint";
+
 import * as parser from "../../workers/parser";
-import { isLoaded, isOriginal } from "../../utils/source";
+import { isLoaded, isOriginal, isPretty } from "../../utils/source";
 import { Telemetry } from "devtools-modules";
 
 import type { ThunkArgs } from "../types";
@@ -30,16 +36,14 @@ async function loadSource(
   text: string,
   contentType: string
 }> {
+  if (isPretty(source) && isOriginal(source)) {
+    const generatedSource = getGeneratedSource(state, source);
+    return prettyPrintSource(sourceMaps, source, generatedSource);
+  }
+
   if (isOriginal(source)) {
     const result = await sourceMaps.getOriginalSourceText(source);
     if (!result) {
-      // TODO: This allows pretty files to continue working the way they have
-      // been, but is very ugly. Remove this when we centralize pretty-printing
-      // in loadSource. https://github.com/firefox-devtools/debugger/issues/8071
-      if (source.isPrettyPrinted) {
-        return null;
-      }
-
       // The way we currently try to load and select a pending
       // selected location, it is possible that we will try to fetch the
       // original source text right after the source map has been cleared

devtools/client/debugger/new/src/actions/sources/prettyPrint.js

@@ -6,11 +6,10 @@
 
 import assert from "../../utils/assert";
 import { recordEvent } from "../../utils/telemetry";
-import { remapBreakpoints, setBreakpointPositions } from "../breakpoints";
+import { remapBreakpoints } from "../breakpoints";
 
 import { setSymbols } from "../ast";
 import { prettyPrint } from "../../workers/pretty-print";
-import { setSource } from "../../workers/parser";
 import { getPrettySourceURL, isLoaded } from "../../utils/source";
 import { loadSourceText } from "./loadSourceText";
 import { mapFrames } from "../pause";
@@ -26,7 +25,31 @@ import {
 
 import type { Action, ThunkArgs } from "../types";
 import { selectSource } from "./select";
-import type { JsSource } from "../../types";
+import type { JsSource, Source } from "../../types";
+
+export async function prettyPrintSource(
+  sourceMaps: any,
+  prettySource: Source,
+  generatedSource: any
+) {
+  const url = getPrettySourceURL(generatedSource.url);
+  const { code, mappings } = await prettyPrint({
+    source: generatedSource,
+    url: url
+  });
+  await sourceMaps.applySourceMap(generatedSource.id, url, code, mappings);
+
+  // The source map URL service used by other devtools listens to changes to
+  // sources based on their actor IDs, so apply the mapping there too.
+  for (const sourceActor of generatedSource.actors) {
+    await sourceMaps.applySourceMap(sourceActor.actor, url, code, mappings);
+  }
+  return {
+    id: prettySource.id,
+    text: code,
+    contentType: "text/javascript"
+  };
+}
 
 export function createPrettySource(sourceId: string) {
   return async ({ dispatch, getState, sourceMaps }: ThunkArgs) => {
@@ -49,26 +72,7 @@ export function createPrettySource(sourceId: string) {
     };
 
     dispatch(({ type: "ADD_SOURCE", source: prettySource }: Action));
-    dispatch(selectSource(prettySource.id));
-
-    const { code, mappings } = await prettyPrint({ source, url });
-    await sourceMaps.applySourceMap(source.id, url, code, mappings);
-
-    // The source map URL service used by other devtools listens to changes to
-    // sources based on their actor IDs, so apply the mapping there too.
-    for (const sourceActor of source.actors) {
-      await sourceMaps.applySourceMap(sourceActor.actor, url, code, mappings);
-    }
-
-    const loadedPrettySource: JsSource = {
-      ...prettySource,
-      text: code,
-      loadedState: "loaded"
-    };
-
-    setSource(loadedPrettySource);
-    dispatch(({ type: "UPDATE_SOURCE", source: loadedPrettySource }: Action));
-    await dispatch(setBreakpointPositions(loadedPrettySource.id));
+    await dispatch(selectSource(prettySource.id));
 
     return prettySource;
   };

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

@@ -320,12 +320,6 @@ function updateLoadedState(
   } else if (action.status === "error") {
     source = { id: sourceId, error: action.error, loadedState: "loaded" };
   } else {
-    // TODO: Remove this once we centralize pretty-print and this can no longer
-    // return a null value when loading a in-progress prettyprinting file.
-    if (!action.value) {
-      return state;
-    }
-
     source = {
       id: sourceId,
       text: action.value.text,