More advanced original source scopes mapping. by yurydelendik · Pull Request #4521 · firefox-devtools/debugger

yurydelendik · 2017-10-27T21:14:22Z

Associated Issue: Display synthesized scopes (e.g. ES6) instead of transpiled

Test case:

Open https://yurydelendik.github.io/old-man-sandbox/sorted-es6/test.html in debugger
Set breakpoint at webpack/sorted.js line 9 (return na < nb ? -1 : na > nb ? 1 : 0;)
Reload the page

jasonLaster

The next step is to add some unit tests, which will help us refactor more confidently and not regress w/ this complicated logic.

I think that these utils might be happier in the mappings library where they can take advantage of the other utils to get good input: i.e.

describe("synthesizeScopes", () => {

  it("basic", () => {
     const fixture = getFixture("sum")
     const mappings ..
     const scopes ...

    // we'll need to fake the frame and pause packets, but we don't have a good mechanism to do that in dbg.html either

    expect(synthesizeScopes(scope, frame, pause)).toMatchSnapshot();
  })

})

jasonLaster

left some style comments…

jasonLaster · 2017-11-07T14:39:23Z


    await dispatch(loadSourceText(generatedSource));
    originalSources.forEach(async source => {
+      await dispatch(loadSourceText(source));


why is this necessary? It could be a very expensive operation

jasonLaster · 2017-11-07T14:39:23Z

+        : L10N.getStr("anonymous");
+  } else {
+    title = L10N.getStr("scopes.block");
+  }


perhaps a small helper for getting the scope title

function getScopeTitle(...) { if (type === "function") { // FIXME Use original function name here const lastGeneratedScope = generatedScopes[generatedScopes.length - 1]; const isLastGeneratedScopeFn = lastGeneratedScope && lastGeneratedScope.type === "function"; title = isLastGeneratedScopeFn && lastGeneratedScope.function.displayName ? simplifyDisplayName(lastGeneratedScope.function.displayName) : L10N.getStr("anonymous"); } else { title = L10N.getStr("scopes.block"); } }

jasonLaster · 2017-11-07T14:39:23Z

+    scopeIndex,
+    lastScopeIndex: syntheticScopes.scopes.length - 1,
+    generatedScopes,
+    foundGeneratedNames: Object.create(null),


style nit: foundGeneratedNames: {}

jasonLaster · 2017-11-07T14:39:23Z

+      path: `${key}/${generatedName}`,
+      contents: { value: { type: "undefined" } }
+    });
+  });


can we move this to a small helper:

function findOriginalBindings(...) { let vars = []; bindingsNames.forEach(name => { // Find binding name in the original source bindings const generatedScope = generatedScopes.find( gs => gs.sourceBindings && name in gs.sourceBindings ); if (!generatedScope || !generatedScope.sourceBindings) { return; } // .. and map it to the generated name const generatedName = generatedScope.sourceBindings[name]; // Skip if we already use the generated name if (generatedName && !foundGeneratedNames[generatedName]) { if (generatedScope.bindings.variables[generatedName]) { vars.push({ name, generatedName, path: `${key}/${generatedName}`, contents: generatedScope.bindings.variables[generatedName] }); foundGeneratedNames[generatedName] = true; return; } const arg = generatedScope.bindings.arguments.find( arg_ => arg_[generatedName] ); if (arg) { vars.push({ name, generatedName, path: `${key}/${generatedName}`, contents: arg[generatedName] }); foundGeneratedNames[generatedName] = true; return; } } vars.push({ name, generatedName, path: `${key}/${generatedName}`, contents: { value: { type: "undefined" } } }); }); }

jasonLaster · 2017-11-07T14:39:23Z

+    });
+  }
+  return acc;
+}


synthesize scope is complicated enough where it takes some time to see what it is trying to do. I think it would be nice to extract the "real" work so that the code flow is more readable.

function synthesizeScope( acc: SynthesizeScopeContext, syntheticScope: SyntheticScope, index: number ): SynthesizeScopeContext { let bindings = findOriginalBindings(...) if (isLast) { bindings = [...bindings, ...findUnusedBindings(...)] } if (isFirst) { bindings = [...bindings, ...getSpecialVariables(...)] // ... } if (bindings.length > 0) { const title = getScopeTitle(...); result.push({ name: title, path: key, contents: vars }); } return acc }

jasonLaster · 2017-11-07T14:39:23Z

+    s = s.parent;
+  }
+
+  const { result } = syntheticScopes.scopes.reduce(synthesizeScope, {


I think the fact that synthesizeScope is mutating makes synthesize a bit more complicated... perhaps, we could have it return the bindings...

const { result } = syntheticScopes.scopes.reduce((bloby, syntheticScope, index) => { const bindings = getBindingsFromScope(bloby,..) // it's tempting to update foundGeneratedNames here too so that `getBindingsFromScope` is pure, // but not sure if that is needed... if (bindings.length) { const title = getScopeTitle(...); blob.result.push({ name: title, path: key, contents: vars }); } }, blobOStuff)

jasonLaster · 2017-11-07T18:00:54Z

+    });
+  }
+  return acc;
+}


It would be nice to create the synthetic scope bindings in the map-bindings space. This would be closer to the data structure we'd get from source-maps

we can add the special vars in the debugger side

jasonLaster

This is looking really good

jasonLaster · 2017-11-08T23:12:53Z

+      return generatedScopes;
+    },
+    async getOriginalSourceScopes(location) {
+      return getScopes(location);


lets first load the source here...

How can we track if we already loaded the source?

we can call await loadSourceText and if it is loaded it will come back quickly

jasonLaster · 2017-11-08T23:12:53Z

    );

-    if (sourceRecord.get("isWasm")) {
+    if (generatedSourceRecord.get("isWasm")) {


should we move this down below add_scopes. can we move this to a different PR? b/c wasm is different and scary for some :)

ryanjduffy · 2017-11-10T00:48:03Z

I ran a quick test with this and wanted to verify what I'm seeing.

ES6 Source:

function test () {
	let a = 1;
	if (a) {
	  const b = 2;
	}
}

document.addEventListener('click', test);

Transpiled source:

'use strict';

function test() {
	var a = 1;
	if (a) {
		var b = 2;
	}
}

document.addEventListener('click', test);

//# sourceMappingURL=scopes-compiled.js.map

When pausing on line 4, const b = 2;, both a and b show within the test scope and only <this> within the if block.

I expected <this> to be restricted to function scope and b to be within the block scope.

Thoughts?

jasonLaster · 2017-11-10T01:15:11Z

@ryanjduffy could you "remix" the dbg example: http://dbg.glitch.me/ or... if it is using babel, publish the example somewhere else...

maybe devtools-examples.

yurydelendik · 2017-11-10T13:43:56Z

maybe devtools-examples.

I would rather to see it as a complete example online with source maps. I wonder if babel spits out names.

jasonLaster · 2017-11-10T14:07:46Z

i'll look for a codepen that has babel

yurydelendik · 2017-11-10T15:21:45Z

I expected to be restricted to function scope and b to be within the block scope.

Based on debugger.html logic at https://github.com/devtools-html/debugger.html/blob/60c59811df6e82ab7831d8faab4cee23ec48bd63/src/utils/scopes.js#L173-L179 we place <this> on the selected scope.

ryanjduffy · 2017-11-10T17:14:21Z

Here's a live version of the sample: https://ryanjduffy.github.io/debugger-examples/scopes/

yurydelendik · 2017-11-10T21:17:26Z

(The devtools/client/shared/source-map/worker.js needs to be updated to address the issue above, works in debugger.html now)

yurydelendik · 2017-11-10T21:29:01Z

Here's a live version of the sample: https://ryanjduffy.github.io/debugger-examples/scopes/

@ryanjduffy the names section is empty at https://ryanjduffy.github.io/debugger-examples/scopes/scopes-compiled.js.map , what did you use to generate the .map file?

ryanjduffy · 2017-11-11T07:29:15Z

Babel cli

jasonLaster · 2017-11-14T00:26:36Z

+  return L10N.getStr("scopes.block");
+}
+
+export function translateScope(


does the name translate mean anything special?

would getScope, which is parallel to getScopes work?

yurydelendik force-pushed the advanced-scopes branch 2 times, most recently from 3a11f5c to 69d857a Compare October 27, 2017 23:42

jasonLaster reviewed Oct 30, 2017

View reviewed changes

yurydelendik force-pushed the advanced-scopes branch from 69d857a to a5b2d25 Compare October 30, 2017 15:12

jasonLaster added the 📚 pr-wip label Oct 30, 2017

yurydelendik force-pushed the advanced-scopes branch 4 times, most recently from 2d67e7a to f89a920 Compare November 1, 2017 22:22

yurydelendik changed the title ~~[WIP] More advanced original source scopes mapping.~~ More advanced original source scopes mapping. Nov 1, 2017

yurydelendik mentioned this pull request Nov 3, 2017

Adds remapScopes utility function. firefox-devtools/devtools-core#780

Merged

yurydelendik force-pushed the advanced-scopes branch from f89a920 to a2ac5d1 Compare November 3, 2017 15:40

yurydelendik mentioned this pull request Nov 6, 2017

Moves updateScopeBindings from debugger.html. firefox-devtools/devtools-core#784

Merged

yurydelendik force-pushed the advanced-scopes branch from a2ac5d1 to 485e63c Compare November 6, 2017 23:42

jasonLaster reviewed Nov 7, 2017

View reviewed changes

yurydelendik mentioned this pull request Nov 7, 2017

[WIP] Display scopes saved in source maps #4613

Closed

jasonLaster reviewed Nov 7, 2017

View reviewed changes

yurydelendik force-pushed the advanced-scopes branch 4 times, most recently from 4ebcba5 to 0578470 Compare November 8, 2017 22:57

jasonLaster reviewed Nov 8, 2017

View reviewed changes

yurydelendik force-pushed the advanced-scopes branch from 0578470 to 89b30ce Compare November 9, 2017 19:55

yurydelendik mentioned this pull request Nov 10, 2017

Records non-modified names in the scopes. firefox-devtools/devtools-core#792

Merged

yurydelendik mentioned this pull request Nov 10, 2017

Recovering empty names section. firefox-devtools/devtools-core#796

Merged

yurydelendik force-pushed the advanced-scopes branch from ca06ec1 to dc0f31a Compare November 11, 2017 02:44

jasonLaster force-pushed the advanced-scopes branch from dc0f31a to e94870d Compare November 13, 2017 19:59

jasonLaster reviewed Nov 14, 2017

View reviewed changes

jasonLaster force-pushed the advanced-scopes branch 4 times, most recently from c89600c to 5ff01ad Compare November 17, 2017 22:41

More advanced original source scopes mapping.

e39844d

jasonLaster force-pushed the advanced-scopes branch from 5ff01ad to 172677a Compare November 17, 2017 22:43

tweak bundle

25c2fe5

jasonLaster force-pushed the advanced-scopes branch from 172677a to 25c2fe5 Compare November 17, 2017 23:02

jasonLaster merged commit 7550b07 into firefox-devtools:master Nov 17, 2017

yurydelendik mentioned this pull request Jan 4, 2018

[WIP] Display original scopes data from source maps. #5021

Closed

Uh oh!

Conversation

yurydelendik commented Oct 27, 2017

Uh oh!

jasonLaster left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jasonLaster left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jasonLaster left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ryanjduffy commented Nov 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jasonLaster commented Nov 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yurydelendik commented Nov 10, 2017

Uh oh!

jasonLaster commented Nov 10, 2017

Uh oh!

yurydelendik commented Nov 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ryanjduffy commented Nov 10, 2017

Uh oh!

yurydelendik commented Nov 10, 2017

Uh oh!

yurydelendik commented Nov 10, 2017

Uh oh!

ryanjduffy commented Nov 11, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jasonLaster left a comment •

edited

Loading

ryanjduffy commented Nov 10, 2017 •

edited

Loading

jasonLaster commented Nov 10, 2017 •

edited

Loading

yurydelendik commented Nov 10, 2017 •

edited

Loading