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

fix(language-service): shorthand syntax with variables #40239

Closed
wants to merge 1 commit into from

Conversation

atscott
Copy link
Contributor

@atscott atscott commented Dec 22, 2020

This commit fixes an issue in the ivy native language service
that caused the logic that finds a target node given a template
position to throw away the results. This happened because the
source span of a variable node in the shorthand structural
directive syntax (i.e. *ngIf=) included the entire binding.

The result was that we would add the variable node to the path and then
later detect that the cursor was outside the key and value spans and
throw away the whole result. In general, we do this because we do not
want to show information when the cursor is between a key/value
(inputA=¦"123"). However, when using the shorthand syntax, we run into
the situation where we can match an AttributeBinding as well as the
vaariable in *ngIf="som¦eValue as myLocalVar". This commit updates the
visitor to retain enough information in the visit path to throw away
invalid targets but keep valid ones if there were multiple results on a
t.Element or t.Template.

@atscott atscott added action: review The PR is still awaiting reviews from at least one requested reviewer area: language-service Issues related to Angular's VS Code language service target: minor This PR is targeted for the next minor release labels Dec 22, 2020
@atscott atscott requested a review from kyliau December 22, 2020 17:20
@ngbot ngbot bot modified the milestone: Backlog Dec 22, 2020
@google-cla google-cla bot added the cla: yes label Dec 22, 2020
packages/language-service/ivy/template_target.ts Outdated Show resolved Hide resolved
packages/language-service/ivy/template_target.ts Outdated Show resolved Hide resolved
@kyliau kyliau removed the action: review The PR is still awaiting reviews from at least one requested reviewer label Dec 22, 2020
This commit fixes an issue in the ivy native language service
that caused the logic that finds a target node given a template
position to throw away the results. This happened because the
source span of a variable node in the shorthand structural
directive syntax (i.e. `*ngIf=`) included the entire binding.

The result was that we would add the variable node to the path and then
later detect that the cursor was outside the key and value spans and
throw away the whole result. In general, we do this because we do not
want to show information when the cursor is between a key/value
(`inputA=¦"123"`). However, when using the shorthand syntax, we run into
the situation where we can match an `AttributeBinding` as well as the
vaariable in `*ngIf="som¦eValue as myLocalVar"`. This commit updates the
visitor to retain enough information in the visit path to throw away
invalid targets but keep valid ones if there were multiple results on a
`t.Element` or `t.Template`.
@atscott atscott added the action: merge The PR is ready for merge by the caretaker label Dec 22, 2020
@atscott atscott added this to PRs In Review in Ivy Language Service Dec 22, 2020
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jan 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: language-service Issues related to Angular's VS Code language service cla: yes target: minor This PR is targeted for the next minor release
Projects
No open projects
Ivy Language Service
  
PRs In Review
Development

Successfully merging this pull request may close these issues.

None yet

2 participants