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

Implement InlineValueProvider for TypeScript #119489

Closed
bpasero opened this issue Mar 22, 2021 · 13 comments · May be fixed by #146608
Closed

Implement InlineValueProvider for TypeScript #119489

bpasero opened this issue Mar 22, 2021 · 13 comments · May be fixed by #146608
Assignees
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues typescript Typescript support issues upstream Issue identified as 'upstream' component related (exists outside of VS Code)

Comments

@bpasero
Copy link
Member

bpasero commented Mar 22, 2021

I debug a test and get this:

image

Lots of inline debug values from a different unrelated test method.

Compared to devtools:

image

@bpasero bpasero added the debug Debug viewlet, configurations, breakpoints, adapter issues label Mar 22, 2021
@isidorn
Copy link
Contributor

isidorn commented Mar 22, 2021

For this we would need the js-debug to use the inline value provider API created by @weinand
Thus forwarding to @weinand and @connor4312

@isidorn isidorn assigned weinand and connor4312 and unassigned isidorn Mar 22, 2021
@connor4312
Copy link
Member

connor4312 commented Mar 22, 2021

Hm, we could parse the source and be smarter about only showing values in current and ancestor scopes.

However this requires parsing TypeScript. Several months ago I actually moved away from using typescript to parse ASTs in favor of Acorn since it's much smaller and faster, and we did not actually need to parse TypeScript in the debugger. Maybe this is something that could live in the TypeScript language features since it already has the typescript package available. I would still be happy to help author it, thoughts @mjbvz?

@weinand
Copy link
Contributor

weinand commented Mar 22, 2021

Yes, that's a good example where VS Code's built-in "inline values" support breaks down.
Especially showing the function's source as its inline value is not really that smart ;-) (see #113987)

@isidorn yes, the now finalised "inline values provider" could help here but that would not provided by "js-debug" but by the TypeScript extension because "inline values provider" are registered for a language not a debugger (as @connor4312 already alluded to).

@connor4312 it would be excellent if you could help author it.

@mjbvz
Copy link
Contributor

mjbvz commented Mar 22, 2021

@connor4312 Should this come from the TS server? The typescript extension does not do any ast processing (and may not easily be able to since we try to strip out most all of the TypeScript files not related to tsserver)

@weinand
Copy link
Contributor

weinand commented Mar 22, 2021

Yes, if a language extension has no direct access to an AST, then it should delegate this functionality to a language server (e.g. by introducing a LSP extension).

@mjbvz
Copy link
Contributor

mjbvz commented Mar 22, 2021

We have this larger issue tracking debug asks from the TS team: #88857

@connor4312 If you can put together a short proposal on what you'd need specifically to implement this—basically here's the information we'll send over, what we need back, and a few illustrative examples of the expected behavior—I can try to get it scheduled for the next typescript version

@connor4312
Copy link
Member

connor4312 commented Mar 25, 2021

Sure:

The InlineValuesProvider API allows languages to provide hints to the debugger what expressions or values to evaluate when debugging, showing them inline as Ben posted. VS Code has a built-in heuristic for these, but languages can provide them more intelligently.

Specifically, for JavaScript, I think the following behaviors would be a good starting point, feel free to add/remove from these suggestions:

  • Inline values should only be provided in the current and parent scopes (solving the original issue)
  • Declarations and assignments assignments should evaluate the declared or assigned expression, bar in bar = foo() or foo in function foo() {}
  • Conditional expressions (ternary or if statements) should evaluate the conditional and none of its children, !foo && bar in if (!foo && bar)
  • Property assignments should be evaluated, foo() in { x: foo() }

The debugger will not cause side-effects during evaluation, so you do not need to be concerned about that. Most of these would go through InlineValueEvaluatableExpression, but should use InlineValueVariableLookup when possible.

I think I will later try to do some magic in js-debug so that evaluatable expressions are always invoked on the right call frame--since V8 optimizes away identifiers that are declared in parent scopes but unused in a child scope, calling everything on the top call frame may not always work.

@connor4312 connor4312 changed the title Debug: inline values is overwhelming Implement InlineValurProvider for TypeScript Mar 25, 2021
@connor4312 connor4312 changed the title Implement InlineValurProvider for TypeScript Implement InlineValueProvider for TypeScript Mar 25, 2021
@connor4312
Copy link
Member

Giving this to you, Matt, since I don't think there's any work to be done on the debug front for now

@connor4312 connor4312 added the typescript Typescript support issues label Mar 25, 2021
@connor4312 connor4312 assigned mjbvz and unassigned connor4312 and weinand Mar 25, 2021
@weinand
Copy link
Contributor

weinand commented Mar 25, 2021

@connor4312 great proposal, thanks a lot!
And if additional information is needed on the InlineValuesContext object or additional parameters on the InlineValueEvaluatableExpression object, just let me know. Those types can be extended easily.

@mjbvz
Copy link
Contributor

mjbvz commented Mar 26, 2021

@connor4312 Thanks. Could you put together a few illustrative examples for the TS team. For a given code snippet for example, here's the complete list of inline values that should be returned.

Also how will source mapping work with all this? Should TS be talking about ranges/vars in the original TS file? Or would this only be run on compiled JS?

@mjbvz mjbvz added this to the April 2021 milestone Mar 26, 2021
@mjbvz mjbvz added the upstream Issue identified as 'upstream' component related (exists outside of VS Code) label Mar 26, 2021
@connor4312
Copy link
Member

Could you put together a few illustrative examples for the TS team. For a given code snippet for example, here's the complete list of inline values that should be returned.

function double(n: number) { // eval('double') -> double()
  return n * 2;
}

let x = 1; // InlineValueVariableLookup for x here

if (x === 2) { // eval(x === 2) -> false
  x *= 2; // this line should NOT be evaluated since it's not the scope or its parents
}

if (x === 1) { // eval(x === 1) -> true
  x *= 2; // InlineValueVariableLookup for x here
  const y = {
    z: x ** 2, // eval(x ** 2) -> 16
  };

  console.log(x); // <-- paused here
}

Also how will source mapping work with all this? Should TS be talking about ranges/vars in the original TS file? Or would this only be run on compiled JS?

They should report this information based on whatever file VS Code requests -- which will be the file the debugger is paused in. They don't have to do anything special to deal with sourcemaps/ranges. I don't think js-debug will either, but if it does then I'll take care of it as the TS language server is unaware of sourcemaps.

@mjbvz
Copy link
Contributor

mjbvz commented Mar 30, 2021

Opened microsoft/TypeScript#43449 to track this upstream

@connor4312 Can you please answer any of the TypeScript team's question about the expected behavior

@mjbvz mjbvz closed this as completed Mar 30, 2021
@mjbvz mjbvz removed this from the April 2021 milestone Mar 30, 2021
@github-actions github-actions bot locked and limited conversation to collaborators May 14, 2021
@mjbvz
Copy link
Contributor

mjbvz commented Mar 24, 2022

@connor4312 I'm assigning you since we decided this would need to come through a TypeScript plugin (which I think we'd have to develop)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues typescript Typescript support issues upstream Issue identified as 'upstream' component related (exists outside of VS Code)
Projects
None yet
5 participants