firefox-devtools · jasonLaster · Nov 3, 2017 · Oct 22, 2017 · Oct 22, 2017 · Oct 29, 2017
diff --git a/flow-typed/debugger-html.js b/flow-typed/debugger-html.js
@@ -44,11 +44,13 @@ declare module "debugger-html" {
     sourceUrl?: string
   };
 
-  declare type ASTLocation = {
+  declare type ASTLocation = {|
     name: ?string,
-    column: ?number,
-    line: number
-  };
+    offset: {
+      column: ?number,
+      line: number
+    }
+  |};
 
   /**
  * Breakpoint
@@ -68,6 +70,17 @@ declare module "debugger-html" {
     condition: ?string
   };
 
+  /**
+ * Breakpoint sync data
+ *
+ * @memberof types
+ * @static
+ */
+  declare type BreakpointSyncData = {
+    previousLocation: Location | null,
+    breakpoint: Breakpoint
+  };
+
   /**
  * Breakpoint Result is the return from an add/modify Breakpoint request
  *
@@ -87,7 +100,7 @@ declare module "debugger-html" {
  */
   declare type PendingBreakpoint = {
     location: PendingLocation,
-    astLocaton: ASTLocation,
+    astLocation: ASTLocation,
     generatedLocation: PendingLocation,
     loading: boolean,
     disabled: boolean,

diff --git a/src/actions/breakpoints/syncBreakpoint.js b/src/actions/breakpoints/syncBreakpoint.js
@@ -1,3 +1,4 @@
+// @flow
 import {
   locationMoved,
   createBreakpoint,
@@ -9,8 +10,19 @@ import {
 import { getGeneratedLocation } from "../../utils/source-maps";
 import { originalToGeneratedId } from "devtools-source-map";
 import { getSource } from "../../selectors";
-
-async function makeScopedLocation({ name, offset }, location, source) {
+import type {
+  BreakpointSyncData,
+  Location,
+  ASTLocation,
+  PendingBreakpoint,
+  SourceId
+} from "debugger-html";
+
+async function makeScopedLocation(
+  { name, offset }: ASTLocation,
+  location: Location,
+  source
+) {
   const scope = await findScopeByName(source, name);
   // fallback onto the location line, if the scope is not found
   // note: we may at some point want to delete the breakpoint if the scope
@@ -25,12 +37,13 @@ async function makeScopedLocation({ name, offset }, location, source) {
 }
 
 function createSyncData(
-  pendingBreakpoint,
-  location,
-  generatedLocation,
-  previousLocation = null
-) {
-  const overrides = { ...pendingBreakpoint, generatedLocation };
+  id: SourceId,
+  pendingBreakpoint: PendingBreakpoint,
+  location: Location,
+  generatedLocation: Location,
+  previousLocation: Location | null = null
+): BreakpointSyncData {
+  const overrides = { ...pendingBreakpoint, generatedLocation, id };
   const breakpoint = createBreakpoint(location, overrides);
 
   assertBreakpoint(breakpoint);
@@ -40,12 +53,12 @@ function createSyncData(
 // we have three forms of syncing: disabled syncing, existing server syncing
 // and adding a new breakpoint
 export async function syncClientBreakpoint(
-  getState,
-  client,
-  sourceMaps,
-  sourceId,
+  getState: Function,
+  client: Object,
+  sourceMaps: Object,
+  sourceId: SourceId,
   pendingBreakpoint: PendingBreakpoint
-) {
+): Promise<BreakpointSyncData> {
   assertPendingBreakpoint(pendingBreakpoint);
 
   const source = getSource(getState(), sourceId).toJS();
@@ -87,7 +100,9 @@ export async function syncClientBreakpoint(
   // early return if breakpoint is disabled or we are in the sameLocation
   // send update only to redux
   if (pendingBreakpoint.disabled || (existingClient && isSameLocation)) {
+    const id = pendingBreakpoint.disabled ? "" : existingClient.id;
     return createSyncData(
+      id,
       pendingBreakpoint,
       scopedLocation,
       scopedGeneratedLocation
@@ -102,20 +117,21 @@ export async function syncClientBreakpoint(
   /** ******* Case 2: Add New Breakpoint ***********/
   // If we are not disabled, set the breakpoint on the server and get
   // that info so we can set it on our breakpoints.
-  const clientBreakpoint = await client.setBreakpoint(
+  const { id, actualLocation } = await client.setBreakpoint(
     scopedGeneratedLocation,
     pendingBreakpoint.condition,
     sourceMaps.isOriginalId(sourceId)
   );
 
   // the breakpoint might have slid server side, so we want to get the location
   // based on the server's return value
-  const newGeneratedLocation = clientBreakpoint.actualLocation;
+  const newGeneratedLocation = actualLocation;
   const newLocation = await sourceMaps.getOriginalLocation(
     newGeneratedLocation
   );
 
   return createSyncData(
+    id,
     pendingBreakpoint,
     newLocation,
     newGeneratedLocation,

diff --git a/src/actions/breakpoints/tests/__snapshots__/syncing.js.snap b/src/actions/breakpoints/tests/__snapshots__/syncing.js.snap
@@ -18,12 +18,15 @@ Object {
       "sourceUrl": "http://localhost:8000/gen.js",
     },
     "hidden": false,
+    "id": "foo",
+    "loading": false,
     "location": Object {
       "column": undefined,
       "line": 3,
       "sourceId": "magic.js",
       "sourceUrl": "http://localhost:8000/examples/magic.js",
     },
+    "text": "",
   },
   "previousLocation": null,
 }
@@ -47,12 +50,15 @@ Object {
       "sourceUrl": "http://localhost:8000/gen.js",
     },
     "hidden": false,
+    "id": "foo",
+    "loading": false,
     "location": Object {
       "column": undefined,
       "line": 12,
       "sourceId": "magic.js",
       "sourceUrl": "http://localhost:8000/examples/magic.js",
     },
+    "text": "",
   },
   "previousLocation": null,
 }
@@ -76,12 +82,15 @@ Object {
       "sourceUrl": "http://localhost:8000/gen.js",
     },
     "hidden": false,
+    "id": "foo",
+    "loading": false,
     "location": Object {
       "column": undefined,
       "line": 3,
       "sourceId": "magic.js",
       "sourceUrl": "http://localhost:8000/examples/magic.js",
     },
+    "text": "",
   },
   "previousLocation": null,
 }
@@ -105,12 +114,15 @@ Object {
       "sourceUrl": "http://localhost:8000/gen.js",
     },
     "hidden": false,
+    "id": "gen.js:5:",
+    "loading": false,
     "location": Object {
       "column": undefined,
       "line": 3,
       "sourceId": "magic.js",
       "sourceUrl": "http://localhost:8000/magic.js",
     },
+    "text": "",
   },
   "previousLocation": Object {
     "column": undefined,

diff --git a/src/actions/breakpoints/tests/syncing.js b/src/actions/breakpoints/tests/syncing.js
@@ -43,6 +43,7 @@ function setBreakpoint(location, condition) {
 }
 
 const clientBreakpoint = {
+  id: "foo",
   actualLocation: {
     sourceUrl: "http://localhost:8000/gen.js",
     sourceId: "gen.js",

diff --git a/src/components/SecondaryPanes/Breakpoints.js b/src/components/SecondaryPanes/Breakpoints.js
@@ -44,7 +44,7 @@ type Props = {
   toggleAllBreakpoints: boolean => void,
   toggleDisabledBreakpoint: number => void,
   setBreakpointCondition: Location => void,
-  toggleConditionalBreakpointPanel: number => void
+  openConditionalPanel: number => void
 };
 
 function isCurrentlyPausedAtBreakpoint(pause, breakpoint) {
@@ -107,7 +107,7 @@ class Breakpoints extends PureComponent<Props> {
       toggleAllBreakpoints,
       toggleDisabledBreakpoint,
       setBreakpointCondition,
-      toggleConditionalBreakpointPanel,
+      openConditionalPanel,
       breakpoints
     } = this.props;
 
@@ -268,7 +268,7 @@ class Breakpoints extends PureComponent<Props> {
       accesskey: addConditionKey,
       click: () => {
         this.selectBreakpoint(breakpoint);
-        toggleConditionalBreakpointPanel(breakpoint.location.line);
+        openConditionalPanel(breakpoint.location.line);
       }
     };
 
@@ -278,7 +278,7 @@ class Breakpoints extends PureComponent<Props> {
       accesskey: editConditionKey,
       click: () => {
         this.selectBreakpoint(breakpoint);
-        toggleConditionalBreakpointPanel(breakpoint.location.line);
+        openConditionalPanel(breakpoint.location.line);
       }
     };
 
@@ -339,10 +339,6 @@ class Breakpoints extends PureComponent<Props> {
     this.props.removeBreakpoint(breakpoint.location);
   }
 
-  toggleConditionalBreakpointPanel(line) {
-    this.props.toggleConditionalBreakpointPanel(line);
-  }
-
   renderBreakpoint(breakpoint) {
     const snippet = breakpoint.text || "";
     const locationId = breakpoint.locationId;

diff --git a/src/utils/breakpoint/astBreakpointLocation.js b/src/utils/breakpoint/astBreakpointLocation.js
@@ -3,7 +3,7 @@
 import { getSymbols } from "../../workers/parser";
 
 import type { Scope, AstPosition } from "../../workers/parser/types";
-import type { Location, Source } from "debugger-html";
+import type { Location, Source, ASTLocation } from "debugger-html";
 
 export function containsPosition(a: AstPosition, b: AstPosition) {
   const startsBefore =
@@ -42,7 +42,10 @@ export function findClosestScope(functions: Scope[], location: Location) {
   }, null);
 }
 
-export async function getASTLocation(source: Source, location: Location) {
+export async function getASTLocation(
+  source: Source,
+  location: Location
+): Promise<ASTLocation> {
   const symbols = await getSymbols(source);
   const functions = [...symbols.functions];
 
@@ -53,13 +56,13 @@ export async function getASTLocation(source: Source, location: Location) {
     const line = location.line - scope.location.start.line;
     return {
       name: scope.name,
-      offset: { line }
+      offset: { line, column: undefined }
     };
   }
   return { name: undefined, offset: location };
 }
 
-export async function findScopeByName(source: Source, name: String) {
+export async function findScopeByName(source: Source, name: ?string) {
   const symbols = await getSymbols(source);
   const functions = symbols.functions;
 

diff --git a/src/utils/breakpoint/index.js b/src/utils/breakpoint/index.js
@@ -103,20 +103,28 @@ export function breakpointExists(state: State, location: Location) {
   return currentBp && !currentBp.disabled;
 }
 
-export function createBreakpoint(location: Location, overrides: Object = {}) {
+export function createBreakpoint(
+  location: Location,
+  overrides: Object = {}
+): Breakpoint {
   const {
     condition,
     disabled,
     hidden,
     generatedLocation,
-    astLocation
+    astLocation,
+    id
   } = overrides;
 
+  const defaultASTLocation = { name: undefined, offset: location };
   const properties = {
+    id,
     condition: condition || null,
     disabled: disabled || false,
     hidden: hidden || false,
-    astLocation: astLocation || { offset: location },
+    loading: false,
+    text: "",
+    astLocation: astLocation || defaultASTLocation,
     generatedLocation: generatedLocation || location,
     location
   };

diff --git a/src/utils/breakpoint/tests/__snapshots__/astBreakpointLocation.spec.js.snap b/src/utils/breakpoint/tests/__snapshots__/astBreakpointLocation.spec.js.snap
@@ -24,6 +24,7 @@ exports[`ast valid location returns name for a nested anon fn as the parent func
 Object {
   "name": "outer",
   "offset": Object {
+    "column": undefined,
     "line": 22,
   },
 }
@@ -33,6 +34,7 @@ exports[`ast valid location returns name for a nested named fn 1`] = `
 Object {
   "name": "inner",
   "offset": Object {
+    "column": undefined,
     "line": 1,
   },
 }
@@ -42,6 +44,7 @@ exports[`ast valid location returns name for an anon fn with a named variable 1`
 Object {
   "name": "globalDeclaration",
   "offset": Object {
+    "column": undefined,
     "line": 1,
   },
 }
@@ -51,6 +54,7 @@ exports[`ast valid location returns the scope and offset 1`] = `
 Object {
   "name": "math",
   "offset": Object {
+    "column": undefined,
     "line": 5,
   },
 }