diff --git a/src/actions/pause/mapScopes.js b/src/actions/pause/mapScopes.js index 5d32f244fa..33b381c69a 100644 --- a/src/actions/pause/mapScopes.js +++ b/src/actions/pause/mapScopes.js @@ -84,7 +84,8 @@ async function buildMappedScopes( const generatedAstBindings = buildGeneratedBindingList( scopes, - generatedAstScopes + generatedAstScopes, + frame.this ); const mappedOriginalScopes = await Promise.all( @@ -93,13 +94,19 @@ async function buildMappedScopes( await Promise.all( Object.keys(item.bindings).map(async name => { - generatedBindings[name] = await findGeneratedBinding( + const binding = item.bindings[name]; + + const result = await findGeneratedBinding( sourceMaps, source, name, - item.bindings[name], + binding, generatedAstBindings ); + + if (result) { + generatedBindings[name] = result; + } }) ); @@ -136,15 +143,23 @@ function generateClientScope( const result = originalScopes .slice(0, -2) .reverse() - .reduce( - (acc, orig, i): OriginalScope => ({ + .reduce((acc, orig, i): OriginalScope => { + const { + // The 'this' binding data we have is handled independently, so + // the binding data is not included here. + // eslint-disable-next-line no-unused-vars + this: _this, + ...variables + } = orig.generatedBindings; + + return { // Flow doesn't like casting 'parent'. parent: (acc: any), actor: `originalActor${i}`, type: orig.type, bindings: { arguments: [], - variables: orig.generatedBindings + variables }, ...(orig.type === "function" ? { @@ -160,9 +175,16 @@ function generateClientScope( } } : null) - }), - globalLexicalScope - ); + }; + }, globalLexicalScope); + + // The rendering logic in getScope 'this' bindings only runs on the current + // selected frame scope, so we pluck out the 'this' binding that was mapped, + // and put it in a special location + const thisScope = originalScopes.find(scope => scope.generatedBindings.this); + if (thisScope) { + result.bindings.this = thisScope.generatedBindings.this; + } return result; } @@ -174,6 +196,16 @@ async function findGeneratedBinding( originalBinding: BindingData, generatedAstBindings: Array ): Promise { + // If there are no references to the implicits, then we have no way to + // even attempt to map it back to the original since there is no location + // data to use. Bail out instead of just showing it as unmapped. + if ( + originalBinding.type === "implicit" && + originalBinding.refs.length === 0 + ) { + return null; + } + const { declarations, refs } = originalBinding; const genContent = await declarations @@ -185,6 +217,15 @@ async function findGeneratedBinding( } const gen = await sourceMaps.getGeneratedLocation(pos.start, source); + const genEnd = await sourceMaps.getGeneratedLocation(pos.end, source); + + // Since the map takes the closest location, sometimes mapping a + // binding's location can point at the start of a binding listed after + // it, so we need to make sure it maps to a location that actually has + // a size in order to avoid picking up the wrong descriptor. + if (gen.line === genEnd.line && gen.column === genEnd.column) { + return null; + } return generatedAstBindings.find( val => @@ -192,7 +233,15 @@ async function findGeneratedBinding( ); }, null); - if (genContent && !genContent.desc) { + if (genContent && genContent.desc) { + return genContent.desc; + } else if (genContent) { + // If there is no descriptor for 'this', then this is not the top-level + // 'this' that the server gave us a binding for, and we can just ignore it. + if (name === "this") { + return null; + } + // If the location is found but the descriptor is not, then it // means that the server scope information didn't match the scope // information from the DevTools parsed scopes. @@ -209,19 +258,6 @@ async function findGeneratedBinding( missingArguments: true } }; - } else if (genContent) { - // If `this` is just mapped back to the same `this`, then - // we don't need to do any mapping for it at all. - // if (name === "this" && !genContent.desc) return null; - if (name === "this" && genContent.name === "this") { - return null; - } - - // If the location is found but the descriptor is not, then this - // there is a bug. TODO to maybe log when this happens or something? - // For now mark these with a special type, but we should - // technically flag them. - return genContent.desc; } // If no location mapping is found, then the map is bad, or @@ -251,24 +287,44 @@ type GeneratedBindingLocation = { function buildGeneratedBindingList( scopes: Scope, - generatedAstScopes: SourceScope[] + generatedAstScopes: SourceScope[], + thisBinding: ?BindingContents ): Array { const clientScopes = []; for (let s = scopes; s; s = s.parent) { clientScopes.push(s); } + // The server's binding data doesn't include general 'this' binding + // information, so we manually inject the one 'this' binding we have into + // the normal binding data we are working with. + const frameThisOwner = generatedAstScopes.find( + generated => "this" in generated.bindings + ); + const generatedBindings = clientScopes .reverse() - .map((s, i) => ({ - generated: generatedAstScopes[generatedAstScopes.length - 1 - i], - client: { - ...s, - bindings: s.bindings - ? Object.assign({}, ...s.bindings.arguments, s.bindings.variables) - : {} + .map((s, i) => { + const generated = generatedAstScopes[generatedAstScopes.length - 1 - i]; + + const bindings = s.bindings + ? Object.assign({}, ...s.bindings.arguments, s.bindings.variables) + : {}; + + if (generated === frameThisOwner && thisBinding) { + bindings.this = { + value: thisBinding + }; } - })) + + return { + generated, + client: { + ...s, + bindings + } + }; + }) .slice(2) .reduce((acc, { client: { bindings }, generated }) => { // If the parser worker's result didn't match the client scopes, diff --git a/src/utils/pause/scopes/getScope.js b/src/utils/pause/scopes/getScope.js index fea4459a5d..8241ac24af 100644 --- a/src/utils/pause/scopes/getScope.js +++ b/src/utils/pause/scopes/getScope.js @@ -7,14 +7,14 @@ import { getBindingVariables } from "./getVariables"; import { getFramePopVariables, getThisVariable } from "./utils"; import { simplifyDisplayName } from "../../frame"; -import type { Frame, Why, Scope } from "debugger-html"; +import type { Frame, Why, Scope, BindingContents } from "debugger-html"; import type { NamedValue } from "./types"; export type RenderableScope = { type: $ElementType, actor: $ElementType, - bindings: $ElementType, + bindings: $ElementType & { this?: ?BindingContents }, parent: ?RenderableScope, object?: ?Object, function?: ?{ @@ -59,7 +59,16 @@ export function getScope( if (isLocalScope) { vars = vars.concat(getFramePopVariables(why, key)); - const this_ = getThisVariable(selectedFrame, key); + let thisDesc_ = selectedFrame.this; + + if ("this" in bindings) { + // The presence of "this" means we're rendering a "this" binding + // generated from mapScopes and this can override the binding + // provided by the current frame. + thisDesc_ = bindings.this ? bindings.this.value : null; + } + + const this_ = getThisVariable(thisDesc_, key); if (this_) { vars.push(this_); diff --git a/src/utils/pause/scopes/utils.js b/src/utils/pause/scopes/utils.js index 1da6b4d93c..aa963ec030 100644 --- a/src/utils/pause/scopes/utils.js +++ b/src/utils/pause/scopes/utils.js @@ -40,9 +40,7 @@ export function getFramePopVariables(why: Why, path: string): NamedValue[] { return vars; } -export function getThisVariable(frame: any, path: string): ?NamedValue { - const this_ = frame.this; - +export function getThisVariable(this_: any, path: string): ?NamedValue { if (!this_) { return null; } diff --git a/src/workers/parser/visitor.js b/src/workers/parser/visitor.js index c1d5d2191e..4cc436da7d 100644 --- a/src/workers/parser/visitor.js +++ b/src/workers/parser/visitor.js @@ -10,6 +10,9 @@ import { isGeneratedId } from "devtools-source-map"; import getFunctionName from "./utils/getFunctionName"; /** + * "implicit" + * Variables added automaticly like "this" and "arguments" + * * "var" * Variables declared with "var" or non-block function declarations * @@ -20,7 +23,7 @@ import getFunctionName from "./utils/getFunctionName"; * Variables declared with "const", imported bindings, or added as const * bindings like inner function expressions and inner class names. */ -export type BindingType = "var" | "const" | "let"; +export type BindingType = "implicit" | "var" | "const" | "let"; export type BindingLocation = { start: Location, @@ -257,6 +260,11 @@ function createParseJSScopeVisitor(sourceId: SourceId): ParseJSScopeVisitor { parent = createTempScope("block", "Lexical Global", parent, location); parent = createTempScope("module", "Module", parent, location); + parent.names.this = { + type: "implicit", + declarations: [], + refs: [] + }; return; } if (path.isFunction()) { @@ -298,6 +306,20 @@ function createParseJSScopeVisitor(sourceId: SourceId): ParseJSScopeVisitor { }; } tree.params.forEach(param => parseDeclarator(param, scope, "var")); + + if (!path.isArrowFunctionExpression()) { + scope.names.this = { + type: "implicit", + declarations: [], + refs: [] + }; + scope.names.arguments = { + type: "implicit", + declarations: [], + refs: [] + }; + } + parent = scope; return; } @@ -387,10 +409,26 @@ function createParseJSScopeVisitor(sourceId: SourceId): ParseJSScopeVisitor { } return; } + if (path.isThisExpression()) { + const scope = findIdentifierInScopes(parent, "this"); + if (scope) { + scope.names.this.refs.push(tree.loc); + } + } if (path.parentPath.isClassProperty({ value: tree })) { savedParents.set(path, parent); parent = createTempScope("block", "Class Field", parent, location); + parent.names.this = { + type: "implicit", + declarations: [], + refs: [] + }; + parent.names.arguments = { + type: "implicit", + declarations: [], + refs: [] + }; return; }