Refactor toggling breakpoints logic

src/actions/breakpoints.js

@@ -9,7 +9,12 @@
  */
 
 import { PROMISE } from "../utils/redux/middleware/promise";
-import { getBreakpoint, getBreakpoints } from "../selectors";
+import {
+  getBreakpoint,
+  getBreakpoints,
+  getSelectedSource,
+  getBreakpointAtLocation
+} from "../selectors";
 import { breakpointExists, createBreakpoint } from "./utils/breakpoints";
 
 import {
@@ -115,6 +120,7 @@ export function addBreakpoint(
 export function disableBreakpoint(location: Location) {
   return ({ dispatch, getState, client }: ThunkArgs) => {
     let bp = getBreakpoint(getState(), location);
+
     if (!bp) {
       throw new Error("attempt to disable a breakpoint that does not exist");
     }
@@ -127,7 +133,6 @@ export function disableBreakpoint(location: Location) {
     const action = {
       type: "DISABLE_BREAKPOINT",
       breakpoint: bp,
-      disabled: true,
       [PROMISE]: client.removeBreakpoint(bp)
     };
 
@@ -239,3 +244,52 @@ export function setBreakpointCondition(
     });
   };
 }
+
+export function toggleBreakpoint(line: number, column?: number) {
+  return ({ dispatch, getState, client, sourceMaps }: ThunkArgs) => {
+    const selectedSource = getSelectedSource(getState());
+    const bp = getBreakpointAtLocation(getState(), { line, column });
+
+    if (bp && bp.loading) {
+      return;
+    }
+
+    if (bp) {
+      // NOTE: it's possible the breakpoint has slid to a column
+      return dispatch(
+        removeBreakpoint({
+          sourceId: bp.location.sourceId,
+          line: bp.location.line,
+          column: column || bp.location.column
+        })
+      );
+    }
+
+    return dispatch(
+      addBreakpoint({
+        sourceId: selectedSource.get("id"),
+        sourceUrl: selectedSource.get("url"),
+        line: line,
+        column: column
+      })
+    );
+  };
+}
+
+export function toggleDisabledBreakpoint(line: number, column?: number) {
+  return ({ dispatch, getState, client, sourceMaps }: ThunkArgs) => {
+    const bp = getBreakpointAtLocation(getState(), { line, column });
+    if (bp && bp.loading) {
+      return;
+    }
+
+    if (!bp) {
+      throw new Error("attempt to disable breakpoint that does not exist");
+    }
+
+    if (!bp.disabled) {
+      return dispatch(disableBreakpoint(bp.location));
+    }
+    return dispatch(enableBreakpoint(bp.location));
+  };
+}

src/actions/tests/__snapshots__/pending-breakpoints.js.snap

@@ -13,7 +13,6 @@ Object {
   "id": "bar.js:7:",
   "loading": false,
   "location": Object {
-    "column": undefined,
     "line": 7,
     "sourceId": "bar.js",
     "sourceUrl": "http://localhost:8000/examples/bar.js",

src/actions/tests/breakpoints.js

@@ -1,4 +1,9 @@
-import { createStore, selectors, actions } from "../../utils/test-head";
+import {
+  createStore,
+  selectors,
+  actions,
+  makeSource
+} from "../../utils/test-head";
 import {
   simulateCorrectThreadClient,
   simpleMockThreadClient
@@ -111,6 +116,81 @@ describe("breakpoints", () => {
     expect(selectors.getBreakpoint(getState(), loc2).disabled).toBe(false);
   });
 
+  it("should toggle a breakpoint at a location", async () => {
+    const { dispatch, getState } = createStore(simpleMockThreadClient);
+
+    await dispatch(actions.newSource(makeSource("foo1")));
+    await dispatch(actions.selectSource("foo1", { line: 1 }));
+
+    await dispatch(actions.toggleBreakpoint(5));
+    await dispatch(actions.toggleBreakpoint(6, 1));
+    expect(
+      selectors.getBreakpoint(getState(), { sourceId: "foo1", line: 5 })
+        .disabled
+    ).toBe(false);
+
+    expect(
+      selectors.getBreakpoint(getState(), {
+        sourceId: "foo1",
+        line: 6,
+        column: 1
+      }).disabled
+    ).toBe(false);
+
+    await dispatch(actions.toggleBreakpoint(5));
+    await dispatch(actions.toggleBreakpoint(6, 1));
+
+    expect(
+      selectors.getBreakpoint(getState(), { sourceId: "foo1", line: 5 })
+    ).toBe(undefined);
+
+    expect(
+      selectors.getBreakpoint(getState(), {
+        sourceId: "foo1",
+        line: 6,
+        column: 1
+      })
+    ).toBe(undefined);
+  });
+
+  it("should disable/enable a breakpoint at a location", async () => {
+    const { dispatch, getState } = createStore(simpleMockThreadClient);
+
+    await dispatch(actions.newSource(makeSource("foo1")));
+    await dispatch(actions.selectSource("foo1", { line: 1 }));
+
+    await dispatch(actions.toggleBreakpoint(5));
+    await dispatch(actions.toggleBreakpoint(6, 1));
+    expect(
+      selectors.getBreakpoint(getState(), { sourceId: "foo1", line: 5 })
+        .disabled
+    ).toBe(false);
+
+    expect(
+      selectors.getBreakpoint(getState(), {
+        sourceId: "foo1",
+        line: 6,
+        column: 1
+      }).disabled
+    ).toBe(false);
+
+    await dispatch(actions.toggleDisabledBreakpoint(5));
+    await dispatch(actions.toggleDisabledBreakpoint(6, 1));
+
+    expect(
+      selectors.getBreakpoint(getState(), { sourceId: "foo1", line: 5 })
+        .disabled
+    ).toBe(true);
+
+    expect(
+      selectors.getBreakpoint(getState(), {
+        sourceId: "foo1",
+        line: 6,
+        column: 1
+      }).disabled
+    ).toBe(true);
+  });
+
   it("should set the breakpoint condition", async () => {
     const { dispatch, getState } = createStore(simpleMockThreadClient);
 

src/actions/tests/helpers/breakpoints.js

@@ -1,5 +1,16 @@
 import { makeLocationId } from "../../../utils/breakpoint";
 
+const sourceFixtures = {
+  foo1: {
+    source: "function foo1() {\n  return 5;\n}",
+    contentType: "text/javascript"
+  },
+  foo2: {
+    source: "function foo2(x, y) {\n  return x + y;\n}",
+    contentType: "text/javascript"
+  }
+};
+
 export function mockPendingBreakpoint(overrides = {}) {
   const { sourceUrl, line, column, condition, disabled } = overrides;
   return {
@@ -65,8 +76,17 @@ export const simpleMockThreadClient = {
   setBreakpoint: (location, _condition) =>
     Promise.resolve({ id: "hi", actualLocation: location }),
 
-  removeBreakpoint: _id => Promise.resolve({ status: "done" }),
+  removeBreakpoint: _id => Promise.resolve(),
 
   setBreakpointCondition: (_id, _location, _condition, _noSliding) =>
-    Promise.resolve({ sourceId: "a", line: 5 })
+    Promise.resolve({ sourceId: "a", line: 5 }),
+
+  sourceContents: sourceId =>
+    new Promise((resolve, reject) => {
+      if (sourceFixtures[sourceId]) {
+        resolve(sourceFixtures[sourceId]);
+      }
+
+      reject(`unknown source: ${sourceId}`);
+    })
 };

src/actions/utils/breakpoints.js

@@ -11,10 +11,7 @@ export async function formatClientBreakpoint(
 
   // make sure that we are re-adding the same type of breakpoint. Column
   // or line
-  const actualLocation = equalizeLocationColumn(
-    clientOriginalLocation,
-    location
-  );
+  const actualLocation = clientOriginalLocation;
 
   // the generatedLocation might have slid, so now we can adjust it
   const generatedLocation = clientBreakpoint.actualLocation;

src/components/Editor/Breakpoint.js

@@ -2,11 +2,12 @@
 import { Component } from "react";
 import { isEnabled } from "devtools-config";
 import ReactDOM from "react-dom";
-import { isGeneratedId } from "devtools-source-map";
 
 import classnames from "classnames";
 import Svg from "../shared/Svg";
 
+import { showSourceText } from "../../utils/editor";
+
 const breakpointSvg = document.createElement("div");
 ReactDOM.render(Svg("breakpoint"), breakpointSvg);
 
@@ -34,39 +35,29 @@ class Breakpoint extends Component {
     this.addBreakpoint = this.addBreakpoint.bind(this);
   }
 
-  isGeneratedSource() {
-    return isGeneratedId(this.props.selectedSource.get("id"));
-  }
-
-  getLocation() {
-    const { breakpoint } = this.props;
-    return this.isGeneratedSource()
-      ? breakpoint.generatedLocation
-      : breakpoint.location;
-  }
-
   addBreakpoint() {
-    const { breakpoint, editor } = this.props;
+    const { breakpoint, editor, selectedSource } = this.props;
 
     // NOTE: we need to wait for the breakpoint to be loaded
     // to get the generated location
-    if (this.isGeneratedSource() && breakpoint.loading) {
+    if (!selectedSource || breakpoint.loading) {
       return;
     }
 
-    const location = this.getLocation();
-    const line = location.line - 1;
+    const line = breakpoint.location.line - 1;
 
-    editor.setGutterMarker(
+    showSourceText(editor, selectedSource.toJS());
+    editor.codeMirror.setGutterMarker(
       line,
       "breakpoints",
       makeMarker(breakpoint.disabled)
     );
-    editor.addLineClass(line, "line", "new-breakpoint");
+
+    editor.codeMirror.addLineClass(line, "line", "new-breakpoint");
     if (breakpoint.condition) {
-      editor.addLineClass(line, "line", "has-condition");
+      editor.codeMirror.addLineClass(line, "line", "has-condition");
     } else {
-      editor.removeLineClass(line, "line", "has-condition");
+      editor.codeMirror.removeLineClass(line, "line", "has-condition");
     }
   }
 
@@ -95,21 +86,20 @@ class Breakpoint extends Component {
 
   componentWillUnmount() {
     const { editor, breakpoint } = this.props;
+
     if (!editor) {
       return;
     }
 
-    const location = this.getLocation();
-
-    if (breakpoint.loading || !location) {
+    if (breakpoint.loading) {
       return;
     }
 
-    const line = location.line - 1;
+    const line = breakpoint.location.line - 1;
 
-    editor.setGutterMarker(line, "breakpoints", null);
-    editor.removeLineClass(line, "line", "new-breakpoint");
-    editor.removeLineClass(line, "line", "has-condition");
+    editor.codeMirror.setGutterMarker(line, "breakpoints", null);
+    editor.codeMirror.removeLineClass(line, "line", "new-breakpoint");
+    editor.codeMirror.removeLineClass(line, "line", "has-condition");
   }
 
   render() {

src/components/Editor/Breakpoints.js

@@ -0,0 +1,67 @@
+import { connect } from "react-redux";
+import { bindActionCreators } from "redux";
+import { Component, createFactory, DOM as dom } from "react";
+import { isEnabled } from "devtools-config";
+
+import _Breakpoint from "./Breakpoint";
+const Breakpoint = createFactory(_Breakpoint);
+
+import actions from "../../actions";
+import { getSelectedSource } from "../../selectors";
+import getVisibleBreakpoints from "../../selectors/visibleBreakpoints";
+import { makeLocationId } from "../../utils/breakpoint";
+
+import type { SourceRecord, BreakpointMap } from "../../reducers/types";
+
+type props = {
+  selectedSource: SourceRecord,
+  breakpoints: BreakpointMap,
+  editor: Object
+};
+
+class Breakpoints extends Component {
+  props: props;
+
+  shouldComponentUpdate(nextProps: any) {
+    if (nextProps.selectedSource && nextProps.selectedSource.get("loading")) {
+      return false;
+    }
+
+    return true;
+  }
+
+  render() {
+    const { breakpoints, selectedSource, editor } = this.props;
+
+    if (!selectedSource || !breakpoints || selectedSource.get("isBlackBoxed")) {
+      return null;
+    }
+
+    return dom.div(
+      {},
+      breakpoints
+        .valueSeq()
+        .filter(
+          b => (isEnabled("columnBreakpoints") ? !b.location.column : true)
+        )
+        .map(bp =>
+          Breakpoint({
+            key: makeLocationId(bp.location),
+            breakpoint: bp,
+            selectedSource,
+            editor: editor
+          })
+        )
+    );
+  }
+}
+
+Breakpoints.displayName = "Breakpoints";
+
+export default connect(
+  state => ({
+    breakpoints: getVisibleBreakpoints(state),
+    selectedSource: getSelectedSource(state)
+  }),
+  dispatch => bindActionCreators(actions, dispatch)
+)(Breakpoints);

src/components/Editor/CallSites.js

@@ -38,6 +38,7 @@ class CallSites extends Component {
     symbols: Array<Symbol>,
     callSites: Array<Symbol>,
     editor: Object,
+    breakpoints: Map,
     addBreakpoint: Function,
     removeBreakpoint: Function,
     selectedSource: Object,