fix(language-service): Do not include $event parameter in reference r… #40158
Conversation
atscott
added
action: review
…esults Given the template `<div (click)="doSomething($event)"></div>` If you request references for the `$event`, the results include both `$event` and `(click)="doSomething($event)"`. This happens because in the TCB, `$event` is passed to the `subscribe`/`addEventListener` function as an argument. So when we ask typescript to give us the references, we get the result from the usage in the subscribe body as well as the one passed in as an argument. This commit adds an identifier to the `$event` parameter in the TCB so that the result returned from `getReferencesAtPosition` can be identified and filtered out. fixes angular#40157
| @@ -107,6 +107,22 @@ describe('find references', () => { | |||
| assertTextSpans(refs, ['title']); | |||
| }); | |||
|
|
|||
There was a problem hiding this comment.
Suggestion
It gets a
$event's reference to itself in a method call argument.
It's more helpful to maintainers to describe what the expected be behavior is, rather than say it should "work" :). What does "it works" mean.
Also, do you know if we have an official guide or code style on writing tests?
There was a problem hiding this comment.
Taking the above describe, the full description of this test would read find references should work for $event in method call arguments. Is that not descriptive enough?
No, there's no explicit style guide for tests.
|
I see that you just added the If you want your PR to be merged, it has to pass all the CI checks. If you can't get the PR to a green state due to flakes or broken master, please try rebasing to master and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help. |
atscott
removed
the
action: review
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
…esults
Given the template
<div (click)="doSomething($event)"></div>If you request references for the
$event, the results include both$eventand(click)="doSomething($event)".This happens because in the TCB,
$eventis passed to thesubscribe/addEventListenerfunction as an argument. So when we ask typescript to give us the references, we
get the result from the usage in the subscribe body as well as the one passed in as an argument.
This commit adds an identifier to the
$eventparameter in the TCB sothat the result returned from
getReferencesAtPositioncan beidentified and filtered out.
fixes #40157