Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

Apply SourceMaps type to redux actions #8077

Merged
merged 8 commits into from
Apr 15, 2019
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions flow-typed/debugger-html.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,18 @@ declare module "debugger-html" {
this: Object
};

/**
* Original Frame
*
* @memberof types
* @static
*/
declare type OriginalFrame = {
displayName: string,
location?: SourceLocation,
thread?: string
};

/**
* why
* @memberof types
Expand Down
29 changes: 11 additions & 18 deletions packages/devtools-source-map/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,13 @@ const {
workerUtils: { WorkerDispatcher }
} = require("devtools-utils");

import type { SourceLocation, Source, SourceId } from "../../../src/types";
import type {
OriginalFrame,
Range,
SourceLocation,
Source,
SourceId
} from "../../../src/types";
import type { SourceMapConsumer } from "source-map";
import type { locationOptions } from "./source-map";

Expand Down Expand Up @@ -87,18 +93,7 @@ export const getGeneratedRangesForOriginal = async (
sourceId: SourceId,
url: string,
mergeUnmappedRegions?: boolean
): Promise<
Array<{
start: {
line: number,
column: number
},
end: {
line: number,
column: number
}
}>
> =>
): Promise<Range[]> =>
dispatcher.invoke(
"getGeneratedRangesForOriginal",
sourceId,
Expand All @@ -108,7 +103,7 @@ export const getGeneratedRangesForOriginal = async (

export const getFileGeneratedRange = async (
originalSource: Source
): Promise<?{ start: any, end: any }> =>
): Promise<{ start: any, end: any }> =>
dispatcher.invoke("getFileGeneratedRange", originalSource);

export const getLocationScopes = dispatcher.task("getLocationScopes");
Expand Down Expand Up @@ -137,10 +132,8 @@ export const hasMappedSource = async (

export const getOriginalStackFrames = async (
generatedLocation: SourceLocation
): Promise<?Array<{
displayName: string,
location?: SourceLocation
}>> => dispatcher.invoke("getOriginalStackFrames", generatedLocation);
): Promise<?Array<OriginalFrame>> =>
dispatcher.invoke("getOriginalStackFrames", generatedLocation);

export {
originalToGeneratedId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,15 @@

// @flow

import type { SourceLocation } from "debugger-html";
import type { OriginalFrame, SourceLocation } from "debugger-html";

const { getWasmXScopes } = require("./wasmXScopes");

// Returns expanded stack frames details based on the generated location.
// The function return null if not information was found.
async function getOriginalStackFrames(
generatedLocation: SourceLocation
): Promise<?Array<{
displayName: string,
location?: SourceLocation
}>> {
): Promise<?Array<OriginalFrame>> {
const wasmXScopes = await getWasmXScopes(generatedLocation.sourceId);
if (!wasmXScopes) {
return null;
Expand Down
9 changes: 2 additions & 7 deletions packages/devtools-source-map/src/utils/wasmXScopes.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
// @flow
/* eslint camelcase: 0*/

import type { SourceLocation, SourceId } from "debugger-html";
import type { OriginalFrame, SourceLocation, SourceId } from "debugger-html";

const { getSourceMap } = require("./sourceMapRequests");
const { generatedToOriginalId } = require("./index");
Expand Down Expand Up @@ -153,12 +153,7 @@ class XScope {
this.xScope = xScopeData;
}

search(
generatedLocation: SourceLocation
): Array<{
displayName: string,
location?: SourceLocation
}> {
search(generatedLocation: SourceLocation): Array<OriginalFrame> {
const { code_section_offset, debug_info, sources, idIndex } = this.xScope;
const pc = generatedLocation.line - (code_section_offset || 0);
const scopes = filterScopes(debug_info, pc, null, idIndex);
Expand Down
12 changes: 8 additions & 4 deletions src/actions/breakpoints/breakpointPositions.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
getBreakpointPositionsForSource
} from "../../selectors";

import type { MappedLocation, SourceLocation } from "../../types";
import type { MappedLocation, Range, SourceLocation } from "../../types";
import type { ThunkArgs } from "../../actions/types";
import { makeBreakpointId } from "../../utils/breakpoint";
import typeof SourceMaps from "../../../packages/devtools-source-map/src";
Expand Down Expand Up @@ -65,7 +65,9 @@ async function _setBreakpointPositions(sourceId, thunkArgs) {

let results = {};
if (isOriginalId(sourceId)) {
const ranges = await sourceMaps.getGeneratedRangesForOriginal(
// Explicitly typing ranges is required to work around the following issue
// https://github.com/facebook/flow/issues/5294
const ranges: Range[] = await sourceMaps.getGeneratedRangesForOriginal(
sourceId,
generatedSource.url,
true
Expand All @@ -81,8 +83,10 @@ async function _setBreakpointPositions(sourceId, thunkArgs) {
// and because we know we don't care about the end-line whitespace
// in this case.
if (range.end.column === Infinity) {
range.end.line += 1;
range.end.column = 0;
range.end = {
line: range.end.line + 1,
column: 0
};
}

const bps = await client.getBreakpointPositions(generatedSource, range);
Expand Down
3 changes: 2 additions & 1 deletion src/actions/breakpoints/remapLocations.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@
// @flow

import type { Breakpoint } from "../../types";
import typeof SourceMaps from "../../../packages/devtools-source-map/src";

export default function remapLocations(
breakpoints: Breakpoint[],
sourceId: string,
sourceMaps: Object
sourceMaps: SourceMaps
) {
const sourceBreakpoints: Promise<Breakpoint>[] = breakpoints.map(
async breakpoint => {
Expand Down
20 changes: 13 additions & 7 deletions src/actions/pause/mapFrames.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@ import {
import assert from "../../utils/assert";
import { findClosestFunction } from "../../utils/ast";

import type { Frame, ThreadId } from "../../types";
import type { Frame, OriginalFrame, ThreadId } from "../../types";
import type { State } from "../../reducers/types";
import type { ThunkArgs } from "../types";

/* eslint-disable import/no-duplicates */
ryanjduffy marked this conversation as resolved.
Show resolved Hide resolved
import { isGeneratedId } from "devtools-source-map";
import typeof SourceMaps from "devtools-source-map";

function isFrameBlackboxed(state, frame) {
const source = getSource(state, frame.location.sourceId);
Expand All @@ -35,7 +37,7 @@ function getSelectedFrameId(state, thread, frames) {
return selectedFrame && selectedFrame.id;
}

export function updateFrameLocation(frame: Frame, sourceMaps: any) {
export function updateFrameLocation(frame: Frame, sourceMaps: SourceMaps) {
if (frame.isOriginal) {
return Promise.resolve(frame);
}
Expand All @@ -48,7 +50,7 @@ export function updateFrameLocation(frame: Frame, sourceMaps: any) {

function updateFrameLocations(
frames: Frame[],
sourceMaps: any
sourceMaps: SourceMaps
): Promise<Frame[]> {
if (!frames || frames.length == 0) {
return Promise.resolve(frames);
Expand Down Expand Up @@ -104,7 +106,7 @@ function isWasmOriginalSourceFrame(frame, getState: () => State): boolean {

async function expandFrames(
frames: Frame[],
sourceMaps: any,
sourceMaps: SourceMaps,
getState: () => State
): Promise<Frame[]> {
const result = [];
Expand All @@ -114,9 +116,9 @@ async function expandFrames(
result.push(frame);
continue;
}
const originalFrames = await sourceMaps.getOriginalStackFrames(
frame.generatedLocation
);
const originalFrames: ?Array<
OriginalFrame
> = await sourceMaps.getOriginalStackFrames(frame.generatedLocation);
if (!originalFrames) {
result.push(frame);
continue;
Expand All @@ -130,6 +132,10 @@ async function expandFrames(
};

originalFrames.forEach((originalFrame, j) => {
if (!originalFrame.location || !originalFrame.thread) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this is the right approach. From what I read in XScope.search(), displayName will always be present and location may be present but thread shouldn't be. However, it was accessed below so perhaps I've misread.

return;
}

// Keep outer most frame with true actor ID, and generate uniquie
// one for the nested frames.
const id = j == 0 ? frame.id : `${frame.id}-originalFrame${j}`;
Expand Down
6 changes: 4 additions & 2 deletions src/actions/sources/newSources.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,10 @@ function loadSourceMap(sourceId: SourceId) {

let urls = null;
try {
const urlInfo = { ...source };
if (!urlInfo.url) {
// Unable to correctly type the result of a spread on a union type.
// See https://github.com/facebook/flow/pull/7298
const urlInfo: Source = { ...(source: any) };
if (!urlInfo.url && typeof urlInfo.introductionUrl === "string") {
// If the source was dynamically generated (via eval, dynamically
// created script elements, and so forth), it won't have a URL, so that
// it is not collapsed into other sources from the same place. The
Expand Down
4 changes: 3 additions & 1 deletion src/actions/sources/prettyPrint.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

// @flow

import typeof SourceMaps from "devtools-source-map";

import assert from "../../utils/assert";
import { recordEvent } from "../../utils/telemetry";
import { remapBreakpoints } from "../breakpoints";
Expand All @@ -28,7 +30,7 @@ import { selectSource } from "./select";
import type { JsSource, Source } from "../../types";

export async function prettyPrintSource(
sourceMaps: any,
sourceMaps: SourceMaps,
prettySource: Source,
generatedSource: any
) {
Expand Down
4 changes: 3 additions & 1 deletion src/actions/types/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

// @flow

import typeof SourceMaps from "devtools-source-map";

import type { WorkerList, MainThread } from "../../types";
import type { State } from "../../reducers/types";
import type { MatchedLocations } from "../../reducers/file-search";
Expand Down Expand Up @@ -34,7 +36,7 @@ export type ThunkArgs = {
dispatch: (action: any) => Promise<any>,
getState: () => State,
client: typeof clientCommands,
sourceMaps: any,
sourceMaps: SourceMaps,
panel: Panel
};

Expand Down
6 changes: 6 additions & 0 deletions src/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,12 @@ export type ChromeFrame = {
location: ?SourceLocation
};

export type OriginalFrame = {
displayName: string,
location?: SourceLocation,
thread: string
};

/**
* ContextMenuItem
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { filterSortedArray } from "./filtering";
import { mappingContains } from "./mappingContains";

import type { Source, SourceLocation, PartialPosition } from "../../../types";
import typeof SourceMaps from "../../../../packages/devtools-source-map/src";

import type { GeneratedBindingLocation } from "./buildGeneratedBindingList";

Expand All @@ -34,7 +35,7 @@ export async function originalRangeStartsInside(
start: SourceLocation,
end: SourceLocation
},
sourceMaps: any
sourceMaps: SourceMaps
) {
const endPosition = await sourceMaps.getGeneratedLocation(end, source);
const startPosition = await sourceMaps.getGeneratedLocation(start, source);
Expand All @@ -58,11 +59,11 @@ export async function getApplicableBindingsForOriginalPosition(
},
bindingType: BindingType,
locationType: BindingLocationType,
sourceMaps: any
sourceMaps: SourceMaps
): Promise<Array<ApplicableBinding>> {
const ranges = await sourceMaps.getGeneratedRanges(start, source);

const resultRanges = ranges.map(mapRange => ({
const resultRanges: GeneratedRange[] = ranges.map(mapRange => ({
start: {
line: mapRange.line,
column: mapRange.columnStart
Expand Down Expand Up @@ -91,8 +92,10 @@ export async function getApplicableBindingsForOriginalPosition(
mappingContains(range, { start: startPosition, end: startPosition }) &&
positionCmp(range.end, endPosition) < 0
) {
range.end.line = endPosition.line;
range.end.column = endPosition.column;
range.end = {
line: endPosition.line,
column: endPosition.column
};
break;
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/utils/pause/mapScopes/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import type {
BindingContents,
ScopeBindings
} from "../../../types";
import typeof SourceMaps from "../../../../packages/devtools-source-map/src";

export type OriginalScope = RenderableScope;

Expand Down Expand Up @@ -200,7 +201,7 @@ function hasLineMappings(ranges) {
function batchScopeMappings(
originalAstScopes: Array<SourceScope>,
source: Source,
sourceMaps: any
sourceMaps: SourceMaps
) {
const precalculatedRanges = new Map();
const precalculatedLocations = new Map();
Expand Down
3 changes: 2 additions & 1 deletion src/utils/pause/mapScopes/rangeMetadata.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { filterSortedArray } from "./filtering";

import type { SourceScope } from "../../../workers/parser";
import type { PartialPosition, Frame, Source } from "../../../types";
import typeof SourceMaps from "../../../../packages/devtools-source-map/src";

type SourceOriginalRange = {
line: number,
Expand All @@ -34,7 +35,7 @@ export async function loadRangeMetadata(
source: Source,
frame: Frame,
originalAstScopes: Array<SourceScope>,
sourceMaps: any
sourceMaps: SourceMaps
): Promise<Array<MappedOriginalRange>> {
const originalRanges: Array<
SourceOriginalRange
Expand Down
6 changes: 3 additions & 3 deletions src/utils/source-maps.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export async function getGeneratedLocation(
state: Object,
source: Source,
location: SourceLocation,
sourceMaps: Object
sourceMaps: SourceMaps
): Promise<SourceLocation> {
if (!isOriginalId(location.sourceId)) {
return location;
Expand Down Expand Up @@ -81,7 +81,7 @@ export async function getMappedLocation(

export async function mapLocation(
state: Object,
sourceMaps: Object,
sourceMaps: SourceMaps,
location: SourceLocation
): Promise<SourceLocation> {
const source = getSource(state, location.sourceId);
Expand All @@ -94,7 +94,7 @@ export async function mapLocation(
return getGeneratedLocation(state, source, location, sourceMaps);
}

return sourceMaps.getOriginalLocation(location, source);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why source was passed here but seems invalid given the signature of getOriginalLocation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like a good catch

return sourceMaps.getOriginalLocation(location);
}

export function isOriginalSource(source: ?Source) {
Expand Down