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

Debugger setting a different 'this' than node.js when using arrow functions #44785

Closed
ejose19 opened this issue Mar 1, 2018 · 12 comments
Closed
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug debug Debug viewlet, configurations, breakpoints, adapter issues verified Verification succeeded
Milestone

Comments

@ejose19
Copy link

ejose19 commented Mar 1, 2018

  • VSCode Version: 1.20
  • OS Version: Ubuntu

Steps to Reproduce:

  1. Create a js file and add this code:
let Person = {
    name: 'Test',
    getNameF: function() {
        console.log(this.name);
    },
    getNameAF: () => {
        console.log(this.name);
    },
};

Person.getNameF();
Person.getNameAF();
  1. Run the code with node and you will get:
Test
undefined

Which implies that the 'this' inside the arrow function is undefined (as should be), however if you add a breakpoints on:

Person.getNameF();
Person.getNameAF();

You will notice the 'this' value in the debugger is the same in both functions! so for some reason it's not binding to node 'this'

Note: Tested on chrome browser debugger and the expected 'this' value (which is undefined) appears for the arrow function, so this problem is related to vs code only.

@weinand weinand added the debug Debug viewlet, configurations, breakpoints, adapter issues label Mar 1, 2018
@weinand
Copy link
Contributor

weinand commented Mar 1, 2018

I've compared the behaviour of VS Code and Chrome Dev Tools.

Here is the Chrome Dev Tools state for the two breakpoints.

Line 4:
2018-03-01_10-58-50

Line 7:
2018-03-01_10-59-36

The Scope view (Variables view in VS Code) shows no difference for both breakpoints in CDT and in VS Code (which seems to be a bug both in VS Code and CDT).

Evaluating "this" in the REPL results in VS Code in this:

2018-03-01_11-06-04

Which is the same as in CDT:

2018-03-01_11-08-31

So, VS Code and CDT behave the same.

But I see a difference when hovering over "this".

In VS Code there are no differences (which is the bug), but in CDT they differ.

The problem is that VS Code implements the hover by matching variables against the information available in the Variables view. Since there are no differences shown in that view the hover does not show any differences either.

If I enable an option in VS Code to implement the hover by using "evaluate", the problem is solved:

2018-03-01_11-18-04

Here is the hover in CDT (and the discrepancy between hover and scope view):

2018-03-01_11-22-56

@roblourens @isidorn should we turn on evaluate based hovers for node-debug?

@isidorn
Copy link
Contributor

isidorn commented Mar 1, 2018

/findDuplicates

@weinand weinand added bug Issue identified by VS Code Team member as probable bug under-discussion Issue is under discussion for relevance, priority, approach labels Mar 2, 2018
@roblourens
Copy link
Member

This is #13967 dupe bot found one but not the original.

It is fixed in Chrome so the fix should be coming down the pipeline to newer node versions very soon.

@roblourens roblourens added the *duplicate Issue identified as a duplicate of another issue(s) label Mar 6, 2018
@roblourens
Copy link
Member

roblourens commented Mar 6, 2018

should we turn on evaluate based hovers for node-debug?

I don't think we should turn it on to work around this one bug, although it's been filed a few times now. I wouldn't want to trigger side effects in getters without an explicit evaluate step like Chrome devtools has.

@weinand weinand removed the under-discussion Issue is under discussion for relevance, priority, approach label Mar 6, 2018
@isidorn
Copy link
Contributor

isidorn commented Mar 6, 2018

@roblourens why don't we turn on evalute on hover at least for insiders to get some feedback before making a final decision?
Imho triggering a getter will be a very rare case, while the evaluate will be much more precise compared to our current heuristic of looking at the variables (ignoring the scope ranges which are wrong most of the time)

@roblourens
Copy link
Member

We can try it. Have there been many issues besides the scope ranges issue?

Is it weird that CDT shows the ReferenceError when evaluating throws?

image

@roblourens
Copy link
Member

roblourens commented Mar 6, 2018

I wouldn't want to trigger side effects in getters without an explicit evaluate step like Chrome devtools has.

I'm wrong, if you hover over something.getter in CDT, there is no explicit step. Only when you hover something and see the getter property. Not sure what the point of having one but not the other is.

In vscode we invoke getter when you hover something.

It's still worth trying though.

@isidorn
Copy link
Contributor

isidorn commented Mar 7, 2018

@roblourens there have not been so many issues lately, but through history I remember people always complaing about this.
Yeah, let's try it out and get feedback for a couple of weeks and than make an informed decision.

Another idea is to do evaluate when our first strategy does not find any results. So to do evaluate as a fallback. We would probably need a new capability for this though.

@roblourens roblourens removed the *duplicate Issue identified as a duplicate of another issue(s) label Mar 7, 2018
@roblourens roblourens added this to the March 2018 milestone Mar 7, 2018
@roblourens roblourens reopened this Mar 7, 2018
roblourens added a commit to microsoft/vscode-chrome-debug-core that referenced this issue Mar 14, 2018
@ejose19
Copy link
Author

ejose19 commented Mar 15, 2018

I updated my vscode insiders to latest version and still get this to reference the object, both by side panel and by hovering. Was this implemented yet?

@ejose19
Copy link
Author

ejose19 commented Mar 15, 2018

Also worth nothing, when I used google chrome my results differ from @weinand, as CDT was showing the correct result in both hovering and side panel.

Image

@roblourens
Copy link
Member

It's not checked in to Insiders yet.

@rebornix rebornix added the verified Verification succeeded label Mar 29, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators Apr 29, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug debug Debug viewlet, configurations, breakpoints, adapter issues verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

5 participants