Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Scopes] Tracking issue for known original scope mapping tasks #5561

Closed
loganfsmyth opened this Issue Feb 28, 2018 · 9 comments

Comments

Projects
None yet
3 participants
@loganfsmyth
Copy link
Contributor

loganfsmyth commented Feb 28, 2018

Not necessarily all required, but a general tracking issue for known tasks around the original scope work

Bugs

  • Exclude Flowtype bindings from original scope binding lists
  • Generate proper scope bindings when bindings are referenced before they are declared
  • Imported bindings that have been optimized out may cause errors
  • Handle Webpack's eval-source-map devtool option (generated code for map runs inside eval())
  • Some mappings include multiple locations, and often the simple binary search in source-map finds one that isn't ideal. We should expose allGeneratedPositionsFor so we can decide for ourselves. PR
  • Resolving generated locations can sometimes result in ranges larger than a single variable. Most commonly, this is if the identifier is the first thing on its own line, because generally the mapped range includes the indentation whitespace
    • this is not mapped in perf.html's TabBar#mouseDownListener #5967
    • Preview/index.js's addExpression (and other destructured/shorthand-referenced bindings) show as (unmapped) #5967
  • Investigate performance of mapping logic
    • Optimize the perf of generated binding search PR
    • Optimize the perf of original binding mapping by batching the lookup into a single call
  • Bindings sometimes show as (unmapped) instead of (optimized out) for module imports. #5984
  • dependencies are unmapped in debugger.html actions/tabs.js:closeTab #6040
    • If a mapping has multiple ranges, we should search for mappings in all of them
  • JSXIdentifier nodes are not included in visitor.js's binding results #6047
  • this is mapped to _this in a class property in this app #6072
  • Expression variable remapping also renames variables that are shadowed by re-declaration in the expression #6087
  • Consider bailing out of mapping if a range has too many applicable bindings #6099
  • 'this' doesn't map fully in arrow functions #6163
  • Handle single-identifier mapped ranges with more broad matching to addresshttps://github.com/Microsoft/TypeScript/issues/22834
    • Attempt #1, which was reverted partially [PR: #6092]
    • Remove overly-broad matching to allow re-applying matching work #6202
  • Rename the various "babel-" files to something more generic, since we're also covering TS? #6229
  • Ensure that regenerator maps yield statements to return statements regenerator issue #342 regenerator PR #344
  • Handle function body scope with lex and nonlex decls #6387
  • Turns out Babel's t.isReferenced (used in the parser worker's visitor.js) isn't great
    • [ foo ] = [] does not recognize foo as a reference
    • foo = 42 is not recognized either
    • { prop: foo } = {} is because it is an object property, even if it is inside a pattern
  • Stepping to the end of a function
  • Use the original file ranges to more effectively exclude whole-line mappings

Features

  • Explore how watch expressions among other things can leverage the original scope data
  • Enable Typescript parsing via Babylon
  • Rewrite expressions in the browser console using original scope maps
    • (1/3) PR
    • (2/3) make the console use the panel helper to map
    • (3/3) Add mochitests for mapped console behavior
  • DECIDED-AGAINST: Create sourcemaps for console rewritten expressions
  • DECIDED-AGAINST: Consider making the console autocomplete take original scopes into account
  • Consider adding support for "sections" in sourcemaps
  • Handle complex expression mappings better
    • We should execute module namespace getters to show the real value. Modules generally use getters for live values, rather than side-effects, so it should be within user expectations to trigger those
    • Some module imports are mapped to function calls instead of property accesses, but we cannot currently call these functions to get their values

Documentation

  • Document the general architecture #6225

UI

  • Allow users to toggle between original and generated scopes inside the Scopes pane

Typescript / Angular

  • Declaration binding identifiers are not mapped consistently Issue Handled in PR #6092, though it would still be good for TS to do better around decorators and exports
  • UNNEEDED: Single-identifier arrow arguments are not mapped to identifiers Issue

Vue

  • Off-by-one (at least) mappings when used with Babel: Issue PR
  • We'll need to enable the existing HTML parser to also run on original .vue files #6167
  • Look into how imports and exports are handled, and ensure that they resolve original bindings properly

EmberJS

  • Ember requires developers to opt into Babel's source-maps, but even when they do they are often ignored: PR
  • Ember appears to a have custom import logic for its own core imports, rewriting them to reference the Ember global. These currently show as (unmapped) and we should see if that is fixable PR
  • Some mappings include multiple locations, and often the simple binary search in source-map finds one that isn't ideal. We should call getAllGeneratedPositions PR

Ecosystem

  • Backport this/arguments/import/inputSourceMap Babel 7 fixes to Babel 6 Babel PR
  • Expand testcases for other bundling/sourcemap generation tooling
    • Webpack
    • Rollup
    • Parcel
    • Explore how we handle minified code
  • Explore non-Babel implementations of imports to see if mappings need improvements
  • Create more testcases around Babel's binding mappings for various complex transformations
@jasonLaster

This comment has been minimized.

Copy link
Contributor

jasonLaster commented Mar 20, 2018

Useful GIF

original preview

@jasonLaster

This comment has been minimized.

Copy link
Contributor

jasonLaster commented Mar 28, 2018

this is not mapped in perf.html TabBar#mouseDownListener

STR:

  1. clone https://github.com/devtools-html/perf.html/
  2. yarn install, yarn start
  3. go to url
  4. Add a breakpoint in tabBar.js#mouseDownListener
  5. click a tab

@jasonLaster

This comment has been minimized.

Copy link
Contributor

jasonLaster commented Apr 19, 2018

STR:

  1. run debugger.html
  2. navigate to localhost:8000
  3. add a breakpoint in actions/tabs.js closeTab
  4. right click on a tab in the launchpad and select close tab

Screen Shot 2018-04-19 at 8.57.50 AM.png


Note: when i paused and resumed i landed in the same spot and saw the generated bindings...

Screen Shot 2018-04-19 at 9.00.12 AM.png

@jasonLaster

This comment has been minimized.

Copy link
Contributor

jasonLaster commented Apr 21, 2018

STR:

  1. go to https://dbg-class-prop-this.glitch.me/
  2. add a breakpoint in app.jsx

@jasonLaster

This comment has been minimized.

Copy link
Contributor

jasonLaster commented May 10, 2018

Found a new case with nightly today.

STR:

  1. open http://localhost:8000/?react_perf&firefox-tab=child1/tab1
  2. navigate to SecondaryPanes/Breakpoint.js
  3. add a breakpoint on 41
  4. add a breakpoint in the debuggee (todomvc)

the missed bindings are: getBreakpointLocation, getBreakpointText, this corresponds with the functions defined in the module scope

Screen Shot 2018-05-10 at 3.51.07 PM.png

@jasonLaster

This comment has been minimized.

Copy link
Contributor

jasonLaster commented May 15, 2018

@loganfsmyth and i looked into it and it looks like it is related to the fact that a function and class are inside of a function body scope

(function() { /* outer anonymous (variable level) */
  /* lexical scope */
  "use strict";

  function getBreakpointText() {
    console.log(2)
  }

  class Breakpoint {
  }

})()

https://gist.github.com/af8e91b7c322d25b1fc122d7d65ffa6f

@loganfsmyth

This comment has been minimized.

Copy link
Contributor Author

loganfsmyth commented May 23, 2018

Closing in favor of #6400

@digitarald

This comment has been minimized.

Copy link
Member

digitarald commented May 29, 2018

@loganfsmyth what are the minimum versions for libraries/tools (babel, ember, etc) that you fixed during this work? I want to make sure to document the work, which would help developers ensure the best debuggable output.

So far I got:

@loganfsmyth

This comment has been minimized.

Copy link
Contributor Author

loganfsmyth commented Jun 27, 2018

@digitarald babel-core@^6.23.3 and @babel/core@^7.0.0-beta.47 are the first versions of their major versions to include all the fixes we've implemented thus far. A few fixes landed earlier in 7.x, but beta.47 includes the most recent fixes we make, which were for Vue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.