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.

Sign up for GitHub

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

Jump to bottom

fix(language-service): Correctly parse inputs and selectors with dollar signs #44268

Closed
wants to merge 1 commit into from
Closed

fix(language-service): Correctly parse inputs and selectors with dollar signs #44268

wants to merge 1 commit into from

Conversation

atscott
Copy link
Contributor

@atscott atscott commented Nov 24, 2021

When we are going to the definition of an input, we find both the
definition of the input and also look for any directives which have
a selector that matches the input. For example:

@Directive({
  selector: '[greeting]'
})
export class MyDir {
  @Input() greeting!: string;
}

With this commit, we now correctly handle the case where inputs and/or
selectors have a dollar sign in them. The dollar sign has special
meaning in CSS, but when we encounter the dollar in a template, we need
to escape it when used as a selector so that it is taken as a dollar
literal rather than a character with special meaning.

Previously, we were not escaping the dollar sign and the CSS parsing
logic would throw an error. The change in this commit prevents that
error from happening, but a try...catch is still added in case there
is another unhandled use-case. If this happens, we do not want the
goToDefinition operation to completely fail.

Fixes angular/vscode-ng-language-service#1574

@atscott atscott added area: language-service Issues related to Angular's VS Code language service target: patch This PR is targeted for the next patch release labels Nov 24, 2021
@ngbot ngbot bot modified the milestone: Backlog Nov 24, 2021
@google-cla google-cla bot added the cla: yes label Nov 24, 2021
@atscott atscott requested a review from JoostK November 29, 2021 17:40
JoostK
JoostK approved these changes Nov 29, 2021
View reviewed changes
Copy link
Member

@JoostK JoostK left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Oh, there is a word missing between "Correctly" and "inputs" in the commit message!

packages/language-service/src/utils.ts Outdated Show resolved Hide resolved
@atscott atscott changed the title fix(language-service): Correctly inputs and selectors with dollar signs fix(language-service): Correctly parse inputs and selectors with dollar signs Nov 29, 2021
@atscott
fix(language-service): Correctly parse inputs and selectors with doll…
c4b176d 
…ar signs

When we are going to the definition of an input, we find _both_ the
definition of the input _and_ also look for any directives which have
a selector that matches the input. For example:

```
@directive({
  selector: '[greeting]'
})
export class MyDir {
  @input() greeting!: string;
}
```

With this commit, we now correctly handle the case where inputs and/or
selectors have a dollar sign in them. The dollar sign has special
meaning in CSS, but when we encounter the dollar in a template, we need
to escape it when used as a selector so that it is taken as a dollar
literal rather than a character with special meaning.

Previously, we were not escaping the dollar sign and the CSS parsing
logic would throw an error. The change in this commit prevents that
error from happening, but a `try...catch` is still added in case there
is another unhandled use-case. If this happens, we do not want the
`goToDefinition` operation to completely fail.

Fixes angular/vscode-ng-language-service#1574
@atscott atscott added action: merge The PR is ready for merge by the caretaker merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note labels Nov 29, 2021
@atscott
Copy link
Contributor Author

atscott commented Nov 29, 2021

merge assistance: unrelated test failures (legacy saucelabs)

@dylhunn
Copy link
Contributor

dylhunn commented Nov 29, 2021

This PR was merged into the repository by commit 3aafa76.

dylhunn pushed a commit that referenced this pull request Nov 29, 2021
@atscott @dylhunn
fix(language-service): Correctly parse inputs and selectors with doll…
cabc178 
…ar signs (#44268)

When we are going to the definition of an input, we find _both_ the
definition of the input _and_ also look for any directives which have
a selector that matches the input. For example:

```
@directive({
  selector: '[greeting]'
})
export class MyDir {
  @input() greeting!: string;
}
```

With this commit, we now correctly handle the case where inputs and/or
selectors have a dollar sign in them. The dollar sign has special
meaning in CSS, but when we encounter the dollar in a template, we need
to escape it when used as a selector so that it is taken as a dollar
literal rather than a character with special meaning.

Previously, we were not escaping the dollar sign and the CSS parsing
logic would throw an error. The change in this commit prevents that
error from happening, but a `try...catch` is still added in case there
is another unhandled use-case. If this happens, we do not want the
`goToDefinition` operation to completely fail.

Fixes angular/vscode-ng-language-service#1574

PR Close #44268
@dylhunn dylhunn closed this in 3aafa76 Nov 29, 2021
dimakuba pushed a commit to dimakuba/angular that referenced this pull request Dec 28, 2021
@atscott @dimakuba
fix(language-service): Correctly parse inputs and selectors with doll…
9131796 
…ar signs (angular#44268)

When we are going to the definition of an input, we find _both_ the
definition of the input _and_ also look for any directives which have
a selector that matches the input. For example:

```
@directive({
  selector: '[greeting]'
})
export class MyDir {
  @input() greeting!: string;
}
```

With this commit, we now correctly handle the case where inputs and/or
selectors have a dollar sign in them. The dollar sign has special
meaning in CSS, but when we encounter the dollar in a template, we need
to escape it when used as a selector so that it is taken as a dollar
literal rather than a character with special meaning.

Previously, we were not escaping the dollar sign and the CSS parsing
logic would throw an error. The change in this commit prevents that
error from happening, but a `try...catch` is still added in case there
is another unhandled use-case. If this happens, we do not want the
`goToDefinition` operation to completely fail.

Fixes angular/vscode-ng-language-service#1574

PR Close angular#44268
@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 Dec 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@JoostK JoostK JoostK approved these changes

Assignees
No one assigned
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 merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: patch This PR is targeted for the next patch release
Projects
None yet
Milestone
Backlog
Development

Successfully merging this pull request may close these issues.

Inputs with $ suffix prevent elements from being recognized
3 participants
@atscott @dylhunn @JoostK