From 9e777b5a3e253fea778d974ab118c746fa6f1635 Mon Sep 17 00:00:00 2001 From: Hubert Boma Manilla Date: Wed, 10 Jan 2018 20:38:45 +0000 Subject: [PATCH] [Preview] Separate out functions for react and immutable (#5014) --- flow-typed/debugger-html.js | 16 +-- src/actions/breakpoints/syncBreakpoint.js | 9 +- src/actions/preview.js | 102 ++++++++---------- .../tests/__snapshots__/preview.spec.js.snap | 87 ++++++++++++--- src/actions/tests/pause.spec.js | 12 ++- src/actions/tests/preview.spec.js | 80 ++++++++++++-- src/client/firefox/commands.js | 5 + src/components/Editor/Preview/Popup.js | 90 +++++++++------- src/components/SecondaryPanes/Breakpoints.js | 3 +- .../SecondaryPanes/Frames/WhyPaused.js | 4 + src/utils/preview.js | 37 +++++++ src/workers/parser/sources.js | 2 +- 12 files changed, 306 insertions(+), 141 deletions(-) create mode 100644 src/utils/preview.js diff --git a/flow-typed/debugger-html.js b/flow-typed/debugger-html.js index 2f721870d7..efbb343e5a 100644 --- a/flow-typed/debugger-html.js +++ b/flow-typed/debugger-html.js @@ -70,17 +70,6 @@ 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 * @@ -224,7 +213,8 @@ declare module "debugger-html" { url: string, fileName: string, message: string, - name: string + name: string, + ownProperties?: Object }; /** @@ -239,7 +229,7 @@ declare module "debugger-html" { frozen: boolean, isGlobal: boolean, ownPropertyLength: number, - preview: PreviewGrip, + preview?: PreviewGrip, sealed: boolean, type: string }; diff --git a/src/actions/breakpoints/syncBreakpoint.js b/src/actions/breakpoints/syncBreakpoint.js index cd3b944ada..b41a5eef6d 100644 --- a/src/actions/breakpoints/syncBreakpoint.js +++ b/src/actions/breakpoints/syncBreakpoint.js @@ -15,13 +15,18 @@ import { getGeneratedLocation } from "../../utils/source-maps"; import { originalToGeneratedId } from "devtools-source-map"; import { getSource } from "../../selectors"; import type { - BreakpointSyncData, Location, ASTLocation, PendingBreakpoint, - SourceId + SourceId, + Breakpoint } from "debugger-html"; +type BreakpointSyncData = { + previousLocation: Location | null, + breakpoint: Breakpoint +}; + async function makeScopedLocation( { name, offset }: ASTLocation, location: Location, diff --git a/src/actions/preview.js b/src/actions/preview.js index d35b3481ca..6ef3dabd20 100644 --- a/src/actions/preview.js +++ b/src/actions/preview.js @@ -1,7 +1,12 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at . */ + // @flow import { findBestMatchExpression } from "../utils/ast"; import { getTokenLocation } from "../utils/editor"; +import { isReactComponent, isImmutable } from "../utils/preview"; import { isGeneratedId } from "devtools-source-map"; import { PROMISE } from "./utils/middleware/promise"; @@ -20,14 +25,41 @@ import { isEqual } from "lodash"; import type { ThunkArgs } from "./types"; import type { AstLocation } from "../workers/parser"; -const extraProps = { - react: { displayName: "this._reactInternalInstance.getName()" }, - immutable: { - isImmutable: exp => `Immutable.Iterable.isIterable(${exp})`, - entries: exp => `${exp}.toJS()`, - type: exp => `${exp}.constructor.name` +async function getReactProps(evaluate) { + const reactDisplayName = await evaluate( + "this._reactInternalInstance.getName()" + ); + + return { + displayName: reactDisplayName.result + }; +} + +async function getImmutableProps(expression: string, evaluate) { + const immutableEntries = await evaluate((exp => `${exp}.toJS()`)(expression)); + + const immutableType = await evaluate( + (exp => `${exp}.constructor.name`)(expression) + ); + + return { + type: immutableType.result, + entries: immutableEntries.result + }; +} + +async function getExtraProps(expression, result, evaluate) { + const props = {}; + if (isReactComponent(result)) { + props.react = await getReactProps(evaluate); } -}; + + if (isImmutable(result)) { + props.immutable = await getImmutableProps(expression, evaluate); + } + + return props; +} export function updatePreview(target: HTMLElement, editor: any) { return ({ dispatch, getState, client, sourceMaps }: ThunkArgs) => { @@ -101,13 +133,10 @@ export function setPreview( await dispatch({ type: "SET_PREVIEW", [PROMISE]: (async function() { - let immutableType = null; - let immutableEntries = null; - const source = getSelectedSource(getState()); const symbols = getSymbols(getState(), source.toJS()); - const found = findBestMatchExpression(symbols, tokenPos, token); + if (!found) { return; } @@ -133,56 +162,19 @@ export function setPreview( } const selectedFrame = getSelectedFrame(getState()); - const { result } = await client.evaluate(expression, { - frameId: selectedFrame.id - }); - - const reactDisplayName = await client.evaluate( - extraProps.react.displayName, - { - frameId: selectedFrame.id - } + const { result } = await client.evaluateInFrame( + selectedFrame.id, + expression ); - const immutable = await client.evaluate( - extraProps.immutable.isImmutable(expression), - { - frameId: selectedFrame.id - } - ); - - if (immutable.result === true) { - immutableEntries = await client.evaluate( - extraProps.immutable.entries(expression), - { - frameId: selectedFrame.id - } - ); - - immutableType = await client.evaluate( - extraProps.immutable.type(expression), - { - frameId: selectedFrame.id - } - ); - } - - const extra = { - react: { - displayName: reactDisplayName.result - }, - immutable: { - isImmutable: - immutable.result && immutable.result.type !== "undefined", - type: immutableType && immutableType.result, - entries: immutableEntries && immutableEntries.result - } - }; - if (result === undefined) { return; } + const extra = await getExtraProps(expression, result, expr => + client.evaluateInFrame(selectedFrame.id, expr) + ); + return { expression, result, diff --git a/src/actions/tests/__snapshots__/preview.spec.js.snap b/src/actions/tests/__snapshots__/preview.spec.js.snap index 97596b8612..ebfdef82a1 100644 --- a/src/actions/tests/__snapshots__/preview.spec.js.snap +++ b/src/actions/tests/__snapshots__/preview.spec.js.snap @@ -1,19 +1,52 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`setPreview member expression 1`] = ` +exports[`setPreview Immutable list 1`] = ` Object { "cursorPos": undefined, - "expression": "this.bazz", + "expression": "list", "extra": Object { "immutable": Object { - "entries": null, - "isImmutable": undefined, - "type": null, + "entries": Object { + "actor": "bazz", + "preview": Object {}, + }, + "type": "Listless", }, - "react": Object { - "displayName": undefined, + }, + "location": Object { + "end": Object { + "column": 4, + "line": 1, + }, + "start": Object { + "column": 0, + "line": 1, + }, + }, + "result": Object { + "actor": "server1.conn34.child2/obj341", + "preview": Object { + "ownProperties": Object { + "__altered": Object {}, + "__hash": Object {}, + "__ownerID": Object {}, + "_root": Object {}, + }, }, }, + "tokenPos": Object { + "column": 4, + "line": 1, + }, + "updating": false, +} +`; + +exports[`setPreview member expression 1`] = ` +Object { + "cursorPos": undefined, + "expression": "this.bazz", + "extra": Object {}, "location": Object { "end": Object { "column": 37, @@ -36,20 +69,46 @@ Object { } `; -exports[`setPreview this 1`] = ` +exports[`setPreview react instance 1`] = ` Object { "cursorPos": undefined, "expression": "this", "extra": Object { - "immutable": Object { - "entries": null, - "isImmutable": undefined, - "type": null, - }, "react": Object { - "displayName": undefined, + "displayName": "Foo", + }, + }, + "location": Object { + "end": Object { + "column": 32, + "line": 1, + }, + "start": Object { + "column": 28, + "line": 1, + }, + }, + "result": Object { + "actor": "server1.conn34.child2/obj341", + "preview": Object { + "ownProperties": Object { + "_reactInternalInstance": Object {}, + }, }, }, + "tokenPos": Object { + "column": 30, + "line": 1, + }, + "updating": false, +} +`; + +exports[`setPreview this 1`] = ` +Object { + "cursorPos": undefined, + "expression": "this", + "extra": Object {}, "location": Object { "end": Object { "column": 32, diff --git a/src/actions/tests/pause.spec.js b/src/actions/tests/pause.spec.js index d8af799ba5..792d4d8519 100644 --- a/src/actions/tests/pause.spec.js +++ b/src/actions/tests/pause.spec.js @@ -2,6 +2,7 @@ import { actions, selectors, createStore, + waitForState, makeSource } from "../../utils/test-head"; @@ -83,19 +84,20 @@ describe("pause", () => { it("resuming - will re-evaluate watch expressions", async () => { const store = createStore(mockThreadClient); - const { dispatch } = store; + const { dispatch, getState } = store; const mockPauseInfo = createPauseInfo(); await dispatch(actions.newSource(makeSource("foo1"))); - await dispatch(actions.addExpression("foo")); + + dispatch(actions.addExpression("foo")); + await waitForState(store, state => selectors.getExpression(state, "foo")); mockThreadClient.evaluate = () => new Promise(r => r("YAY")); await dispatch(actions.paused(mockPauseInfo)); await dispatch(actions.resumed()); - expect(selectors.getExpression(store.getState(), "foo").value).toEqual( - "YAY" - ); + const expression = selectors.getExpression(getState(), "foo"); + expect(expression.value).toEqual("YAY"); }); }); }); diff --git a/src/actions/tests/preview.spec.js b/src/actions/tests/preview.spec.js index 36dcf5e828..7f50c0bc46 100644 --- a/src/actions/tests/preview.spec.js +++ b/src/actions/tests/preview.spec.js @@ -24,55 +24,113 @@ const threadClient = { return new Promise((resolve, reject) => resolve({ result: evaluationResult[expression] }) ); + }, + evaluateInFrame: function(frameId, expression) { + return new Promise((resolve, reject) => + resolve({ result: evaluationResult[expression] }) + ); } }; const sourceTexts = { "base.js": "function base(boo) {}", "foo.js": "function base(boo) { return this.bazz; } outOfScope", + "immutable.js": "list", "scopes.js": readFixture("scopes.js"), "reactComponent.js": readFixture("reactComponent.js") }; -const evaluationResult = { - "this.bazz": { actor: "bazz", preview: {} }, - this: { actor: "this", preview: {} } +const react = { + actor: "server1.conn34.child2/obj341", + preview: { + ownProperties: { + _reactInternalInstance: {} + } + } }; +const emptyObject = { actor: "this", preview: {} }; + +const immutableList = { + actor: "server1.conn34.child2/obj341", + preview: { + ownProperties: { + _root: {}, + __ownerID: {}, + __altered: {}, + __hash: {} + } + } +}; + +let evaluationResult; + describe("setPreview", () => { let dispatch = undefined; let getState = undefined; - beforeEach(async () => { + async function setup(fileName) { const store = createStore(threadClient); prefs.autoPrettyPrint = false; dispatch = store.dispatch; getState = store.getState; - const foo = makeSource("foo.js"); - await dispatch(actions.newSource(foo)); - await dispatch(actions.loadSourceText(I.Map({ id: "foo.js" }))); + const source = makeSource(fileName); + await dispatch(actions.newSource(source)); + await dispatch(actions.loadSourceText(I.Map({ id: fileName }))); - await dispatch(actions.selectLocation({ sourceId: "foo.js" })); - await dispatch(actions.setSymbols("foo.js")); + await dispatch(actions.selectLocation({ sourceId: fileName })); + await dispatch(actions.setSymbols(fileName)); await dispatch( actions.paused({ why: { type: "resumeLimit" }, - frames: [{ id: "frame1", location: { sourceId: "foo.js" } }] + frames: [{ id: "frame1", location: { sourceId: fileName } }] }) ); - }); + } it("member expression", async () => { + await setup("foo.js"); + + evaluationResult = { "this.bazz": { actor: "bazz", preview: {} } }; await dispatch(actions.setPreview("bazz", { line: 1, column: 34 })); const preview = selectors.getPreview(getState()); expect(preview).toMatchSnapshot(); }); it("this", async () => { + await setup("foo.js"); + + evaluationResult = { this: emptyObject }; + await dispatch(actions.setPreview("this", { line: 1, column: 30 })); + const preview = selectors.getPreview(getState()); + expect(preview).toMatchSnapshot(); + }); + + it("react instance", async () => { + await setup("foo.js"); + evaluationResult = { + this: react, + "this._reactInternalInstance.getName()": "Foo" + }; + await dispatch(actions.setPreview("this", { line: 1, column: 30 })); const preview = selectors.getPreview(getState()); expect(preview).toMatchSnapshot(); }); + + it("Immutable list", async () => { + await setup("immutable.js"); + + evaluationResult = { + list: immutableList, + "list.constructor.name": "Listless", + "list.toJS()": { actor: "bazz", preview: {} } + }; + + await dispatch(actions.setPreview("list", { line: 1, column: 4 })); + const preview = selectors.getPreview(getState()); + expect(preview).toMatchSnapshot(); + }); }); diff --git a/src/client/firefox/commands.js b/src/client/firefox/commands.js index e03c93d70c..ceef74ffcb 100644 --- a/src/client/firefox/commands.js +++ b/src/client/firefox/commands.js @@ -169,6 +169,10 @@ type EvaluateParam = { frameId?: FrameId }; +function evaluateInFrame(frameId: string, script: Script) { + return evaluate(script, { frameId }); +} + function evaluate(script: Script, { frameId }: EvaluateParam): Promise { const params = frameId ? { frameActor: frameId } : {}; if (!tabTarget || !tabTarget.activeConsole) { @@ -307,6 +311,7 @@ const clientCommands = { removeBreakpoint, setBreakpointCondition, evaluate, + evaluateInFrame, debuggeeCommand, navigate, reload, diff --git a/src/components/Editor/Preview/Popup.js b/src/components/Editor/Preview/Popup.js index 191d8bdc38..ea7660e890 100644 --- a/src/components/Editor/Preview/Popup.js +++ b/src/components/Editor/Preview/Popup.js @@ -17,6 +17,7 @@ import { getLoadedObjects } from "../../../selectors"; import Popover from "../../shared/Popover"; import PreviewFunction from "../../shared/PreviewFunction"; import { markText } from "../../../utils/editor"; +import { isReactComponent, isImmutable } from "../../../utils/preview"; import Svg from "../../shared/Svg"; import "./Popup.css"; @@ -38,10 +39,6 @@ type Props = { extra: Object }; -function isReactComponent(roots) { - return roots.some(root => root.name === "_reactInternalInstance"); -} - export class Popup extends Component { marker: any; pos: any; @@ -102,53 +99,70 @@ export class Popup extends Component { ); } + renderReact(react: Object, roots: Array) { + const reactHeader = react.displayName || "React Component"; + + const header = ( +
+

{reactHeader}

+
+ ); + + roots = roots.filter(r => ["state", "props"].includes(r.name)); + return ( +
+ {header} + {this.renderObjectInspector(roots)} +
+ ); + } + + renderImmutable(immutable: Object, roots: Array) { + const immutableHeader = immutable.type || "Immutable"; + + const header = ( +
+ +

{immutableHeader}

+
+ ); + + roots = [ + { + path: "entries", + contents: { value: immutable.entries } + } + ]; + + return ( +
+ {header} + {this.renderObjectInspector(roots)} +
+ ); + } + renderObjectPreview(expression: string, root: Object, extra: Object) { - let header = null; const { loadedObjects } = this.props; const { extra: { react, immutable } } = this.props; const getObjectProperties = id => loadedObjects[id]; - let roots = this.getChildren(root, getObjectProperties); + const roots = this.getChildren(root, getObjectProperties); + const grip = root.contents.value; if (!roots) { return null; } - if (isReactComponent(roots)) { - const reactHeader = react.displayName || "React Component"; - - header = ( -
-

{reactHeader}

-
- ); - - roots = roots.filter(r => ["state", "props"].includes(r.name)); + if (isReactComponent(grip)) { + return this.renderReact(react, roots); } - if (immutable.isImmutable) { - const immutableHeader = immutable.type || "Immutable"; - - header = ( -
- -

{immutableHeader}

-
- ); - - roots = roots.filter(r => ["size"].includes(r.name)); - - roots.push({ - name: "entries", - contents: { value: immutable.entries }, - path: "entries" - }); + if (isImmutable(grip)) { + return this.renderImmutable(immutable, roots); } return ( -
- {header} - {this.renderObjectInspector(roots)} -
+
{this.renderObjectInspector(roots)}
); } @@ -165,7 +179,7 @@ export class Popup extends Component { ); } - renderObjectInspector(roots: Object) { + renderObjectInspector(roots: Array) { const { loadObjectProperties, loadedObjects, openLink } = this.props; const getObjectProperties = id => loadedObjects[id]; diff --git a/src/components/SecondaryPanes/Breakpoints.js b/src/components/SecondaryPanes/Breakpoints.js index ba1ce96f11..b1acbfb0da 100644 --- a/src/components/SecondaryPanes/Breakpoints.js +++ b/src/components/SecondaryPanes/Breakpoints.js @@ -70,8 +70,7 @@ function getBreakpointFilename(source) { function renderSourceLocation(source, line, column) { const filename = getBreakpointFilename(source); const isWasm = source && source.get("isWasm"); - const columnVal = - features.columnBreakpoints && column ? `:${column}` : ""; + const columnVal = features.columnBreakpoints && column ? `:${column}` : ""; const bpLocation = isWasm ? `0x${line.toString(16).toUpperCase()}` : `${line}${columnVal}`; diff --git a/src/components/SecondaryPanes/Frames/WhyPaused.js b/src/components/SecondaryPanes/Frames/WhyPaused.js index b2f5c25625..37a306f213 100644 --- a/src/components/SecondaryPanes/Frames/WhyPaused.js +++ b/src/components/SecondaryPanes/Frames/WhyPaused.js @@ -16,6 +16,10 @@ function renderExceptionSummary(exception: string | Grip) { } const preview = exception.preview; + if (!preview) { + return; + } + return `${preview.name}: ${preview.message}`; } diff --git a/src/utils/preview.js b/src/utils/preview.js new file mode 100644 index 0000000000..31102734ea --- /dev/null +++ b/src/utils/preview.js @@ -0,0 +1,37 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at . */ + +// @flow + +import type { Grip } from "debugger-html"; + +const IMMUTABLE_FIELDS = ["_root", "__ownerID", "__altered", "__hash"]; + +export function isImmutable(result: Grip) { + if (!result || !result.preview) { + return; + } + + const ownProperties = result.preview.ownProperties; + if (!ownProperties) { + return; + } + + return IMMUTABLE_FIELDS.every(field => + Object.keys(ownProperties).includes(field) + ); +} + +export function isReactComponent(result: Grip) { + if (!result || !result.preview) { + return; + } + + const ownProperties = result.preview.ownProperties; + if (!ownProperties) { + return; + } + + return Object.keys(ownProperties).includes("_reactInternalInstance"); +} diff --git a/src/workers/parser/sources.js b/src/workers/parser/sources.js index 94cdea2e2a..c02c3fc59d 100644 --- a/src/workers/parser/sources.js +++ b/src/workers/parser/sources.js @@ -18,7 +18,7 @@ export function setSource(source: Source) { export function getSource(sourceId: SourceId): Source { if (!cachedSources.has(sourceId)) { - throw new Error(`${sourceId} was not provided.`); + throw new Error(`Parser: ${sourceId} was not provided.`); } return ((cachedSources.get(sourceId): any): Source); }