Skip to content
This repository was archived by the owner on Oct 2, 2021. It is now read-only.

Make sourcemap bias check order consistent #394

Open
mrcrane opened this issue Feb 7, 2019 · 13 comments
Open

Make sourcemap bias check order consistent #394

mrcrane opened this issue Feb 7, 2019 · 13 comments
Assignees

Comments

@mrcrane
Copy link
Contributor

mrcrane commented Feb 7, 2019

I'm running into an issue where I am unable to set a breakpoints that are requested on one line of code are "moving" to the next line of code. It makes it impossible to set a breakpoint on the first line of code. I traced the issue to the sourcemap library, which is returning a mapping to the next line. While poking around there, I discovered that we use GREATEST_LOWER_BOUND in authoredPositionFor and that we use LEAST_UPPER_BOUND in generatedPositionFor

Changing generatedPositionFor to check GREATEST_UPPER_BOUND first resolved my issue and I haven't seen any bad effects otherwise.

Looking into the Github history I see that both methods originally checked LEAST_UPPER_BOUND until this fix was put into place 4bb4a8c which only changed one of them. This is related to #112 and also Microsoft/vscode-chrome-debug#417

I'm going to continue to dig into exactly when and how my bug is occurring and also the impact of this potential fix. In the meantime I wanted to start this discussion to understand if there is a reason for the inconsistency and if there is any reason not to make the biases consistent.

@roblourens
Copy link
Member

Haven't thought about this in awhile but the best I can remember is that there are problems with using either bias first and we probably want an even smarter way of deciding which mapping is the best.

I think when doing generatedPositionFor we often have a breakpoint at column 0 and want to get the mapping at column 4 or wherever the code is, and don't want the last mapping from the previous line, so we first try least upper which looks forward.

Then when doing authoredPositionFor, I think that when stepping through multiple expressions on one line, we tend to end up on columns in the middle of the line where we need to look left to find the correct sourcemapping. Otherwise we would end up with a mapping on the next line, or the wrong part of the line.

I'd be happy to come look at what's going on in your case

@mrcrane
Copy link
Contributor Author

mrcrane commented Feb 7, 2019

What you're saying makes some sense based on the unit tests for this code. After changing the bias, breakpoints on column 0 in the authored code are mapping to the last position on the previous line in the generated code which doesn't seem right.

I'm seeing the bug in Visual Studio (reported through Visual Studio community feedback), but now that I'm hooking up the same project through VS Code the breakpoints are working correctly... so this is perhaps related to the way Visual Studio is interacting with the debug adapter. I will investigate further and let you know what I find.

I also stumbled on this issue which is reporting a very similar issue with breakpoints in vue.js: vuejs/vue-loader#1163 Although, Chrome devtools is working properly for my project.

@roblourens
Copy link
Member

I think the vue issue was just that the sourcemaps were not valid. With VS, check that the line numbers are not off by 1, and that the column numbers are the same as vscode, I guess.

@mrcrane
Copy link
Contributor Author

mrcrane commented Feb 7, 2019

So, the difference between VS and VS Code is that when calling setBreakpoints:

  • VS Code will only send the line number for the authored file. The sourcemaps code will then set the column number to zero as a default.
  • VS will send a line number (same line number as above) and also a column number (in my example: column 11 which corresponds to the first column that after the leading indentation whitespace)

This is resulting in different sourcemap results

@roblourens
Copy link
Member

Is it possible that the column is off by one (0 indexed vs 1 indexed) or something? Otherwise I wouldn't expect it to change the behavior. Or maybe there is an off-by-one bug.

We ask the runtime for a list of possible breakpoint locations on that line and that is another place where we have to compare positions and pick the appropriate one so there could be an issue there too.

@roblourens
Copy link
Member

roblourens commented Feb 7, 2019

Another thing that I should have mentioned to help test/verify this sort of thing, you can set breakpoints on a column in vscode (called inline breakpoints) with shift+f9. So you could do that on the first non-whitespace column and check whether it's the same as VS.

@mrcrane
Copy link
Contributor Author

mrcrane commented Feb 8, 2019

Cool! And yes, setting the breakpoint on the same column has the same behavior of skipping to the next line.

@DubbleClick
Copy link

DubbleClick commented Feb 10, 2019

I'm the one opening the issue on the developer community forum, if I can be of any help during this just let me know. I haven't tested it with VS code yet, can however confidently say that using source maps which doesn't include column, but only line information, produce the same behaviour of breakpoints being created 1-2 lines ahead.

@roblourens
Copy link
Member

Thanks @DubbleClick, the best workaround is to use sourcemaps that also include column info, since supporting less precise sourcemaps will take some effort on our end.

@DubbleClick
Copy link

That's alright with me, the issue does however still exist. Visual Studio won't even let me set column specific breakpoints (while Chrome does) and putting a breakpoint in Visual Studio will create it one line ahead inside a function, or two lines into the function if set on the line of the function definition.

@dhanvikapila
Copy link
Member

@roblourens , @mrcrane have we reached out to the Vue community to understand if they can make a fix to the way source maps are generated?

@mrcrane
Copy link
Contributor Author

mrcrane commented Feb 27, 2019

Update on this: we have reproduced this issue on React.js as well. You can see it happen by creating an ASP.NET Core 2.2 web application with React.js. Setting breakpoints in the JSX files shows the same behavior for the same reason. So, this is not limited to Vue.js unfortunately.

We are exploring ways to solve this on the VS side. There is a lot going on with calculating breakpoint locations in VS through the language service that happens before the debug adapter is even involved.

We will also look into ways to handle these line-based source maps better.

@rvnlord
Copy link

rvnlord commented Jan 11, 2020

Steps to reproduce:

  1. Open any vue project (the one from the default vs template is fine)

  2. Place a breakpoint anywhere in client side javascript code

  3. Start debugging

  4. Breakpoint won't be hit

This is not happening when using chrome debugger or when using VS Code with the following recipe: https://github.com/Microsoft/vscode-recipes/blob/master/vuejs-cli/README.md, speciffically you need to override map paths for the debugger to work correctly:

"sourceMapPathOverrides": {              
    "webpack:///./src/*": "${webRoot}/*",              
    "webpack:///src/*": "${webRoot}/*",              
    "webpack:///*": "*",              
    "webpack:///./~/*":      
    "${webRoot}/node_modules/*"        
}

I understand that as of current version of Visual Studio v16.4.5 this setup is not possible in VS 2019. WI ould very much appreciate if you could fix it, because despite people generally using VS Code for developing Vue Apps, I very much prefer VS 2019.
All that is needed is the ability to override map paths in VS 2019.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants