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

feat(language-service): add definitions for templateUrl #32238

Closed
wants to merge 7 commits into from

Conversation

@ayazhafiz
Copy link
Contributor

commented Aug 21, 2019

Adds support for getDefinitionAt when called on a templateUrl
property assignment.

The currrent architecture for getting definitions is designed to be
called on templates, so we have to introduce a new
getTsDefinitionAndBoundSpan method to get Angular-specific definitions
in TypeScript files and pass the TypeScript language service host to this
new method so that URLs can be verified to exist.

Closes angular/vscode-ng-language-service#111
Closes #13888
Closes #13890

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Issue Number: angular/vscode-ng-language-service#111

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@ayazhafiz ayazhafiz requested a review from kyliau Aug 21, 2019

@ayazhafiz ayazhafiz requested a review from angular/tools-language-service as a code owner Aug 21, 2019

@ayazhafiz ayazhafiz self-assigned this Aug 21, 2019

@ngbot ngbot bot modified the milestone: needsTriage Aug 21, 2019

@googlebot googlebot added the cla: yes label Aug 21, 2019

@kyliau
Copy link
Member

left a comment

Could you also please add a test to integration/language-service-plugin?

packages/language-service/src/definitions.ts Outdated Show resolved Hide resolved

@ayazhafiz ayazhafiz force-pushed the ayazhafiz:feat/ls-templateurl branch from 351317c to 0957bfc Aug 22, 2019

ayazhafiz added a commit to ayazhafiz/angular that referenced this pull request Aug 22, 2019
refactor(language-service): add generic decorator property verifications
This PR makes finding class declarations properties in decorators are
applied to more generic to all properties that may be in a decorator,
and adds helper methods enabling getting the property assignment of a
property value and verifying that a property assignment is actually in a
decorator applied to a class.

This is done so that it will be easier to provide Angular definitions
for decorator properties moving forward. Most immediately, this will
provide decorator class verification for angular#32238.
ayazhafiz added a commit to ayazhafiz/angular that referenced this pull request Aug 22, 2019
refactor(language-service): add generic decorator property verifications
This PR makes finding class declarations properties in decorators are
applied to more generic to all properties that may be in a decorator,
and adds helper methods enabling getting the property assignment of a
property value and verifying that a property assignment is actually in a
decorator applied to a class.

This is done so that it will be easier to provide Angular definitions
for decorator properties moving forward. Most immediately, this will
provide decorator class verification for angular#32238.
@ayazhafiz

This comment has been minimized.

Copy link
Contributor Author

commented Aug 22, 2019

Blocked by #32252

atscott added a commit that referenced this pull request Aug 22, 2019
refactor(language-service): add generic decorator property verificati…
…ons (#32252)

This PR makes finding class declarations properties in decorators are
applied to more generic to all properties that may be in a decorator,
and adds helper methods enabling getting the property assignment of a
property value and verifying that a property assignment is actually in a
decorator applied to a class.

This is done so that it will be easier to provide Angular definitions
for decorator properties moving forward. Most immediately, this will
provide decorator class verification for #32238.

PR Close #32252

@ayazhafiz ayazhafiz force-pushed the ayazhafiz:feat/ls-templateurl branch from 0957bfc to 1f4b8b3 Aug 22, 2019

@ayazhafiz ayazhafiz requested a review from angular/fw-integration as a code owner Aug 22, 2019

@ayazhafiz ayazhafiz requested a review from kyliau Aug 22, 2019

ayazhafiz added 3 commits Aug 20, 2019
feat(language-service): add definitions for templateUrl
Adds support for `getDefinitionAt` when called on a templateUrl
property assignment.

The currrent architecture for getting definitions is designed to be
called on templates, so we have to introduce a new
`getTsDefinitionAndBoundSpan` method to get Angular-specific definitions
in TypeScript files and pass a `readTemplate` closure that will read the
contents of a template using `TypeScriptServiceHost#getTemplates`. We
can probably go in and make this nicer in a future PR, though I'm not
sure what the best architecture should be yet.

Part of angular/vscode-ng-language-service#111

@ayazhafiz ayazhafiz force-pushed the ayazhafiz:feat/ls-templateurl branch from 1f4b8b3 to b421b8f Aug 26, 2019

ayazhafiz added 2 commits Aug 26, 2019

@ayazhafiz ayazhafiz requested a review from josephperrott Aug 26, 2019

ngdevelop-tech added a commit to ngdevelop-tech/angular that referenced this pull request Aug 27, 2019
refactor(language-service): add generic decorator property verificati…
…ons (angular#32252)

This PR makes finding class declarations properties in decorators are
applied to more generic to all properties that may be in a decorator,
and adds helper methods enabling getting the property assignment of a
property value and verifying that a property assignment is actually in a
decorator applied to a class.

This is done so that it will be easier to provide Angular definitions
for decorator properties moving forward. Most immediately, this will
provide decorator class verification for angular#32238.

PR Close angular#32252
@kyliau
Copy link
Member

left a comment

Please fix failed test

@kyliau

This comment has been minimized.

Copy link
Member

commented Aug 27, 2019

Adds support for getDefinitionAt when called on a templateUrl
property assignment.

The currrent architecture for getting definitions is designed to be
called on templates, so we have to introduce a new
getTsDefinitionAndBoundSpan method to get Angular-specific definitions
in TypeScript files and pass the TypeScript language service host to this
new method so that URLs can be verified to exist.

Part of angular/vscode-ng-language-service#111

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Issue Number: angular/vscode-ng-language-service#111

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Please include the following snippet in your PR description so that related issues get closed automatically PR closes https://github.com/angular/vscode-ng-language-service/issues/111

Other similar issues:
#13888
#13890

ayazhafiz added 2 commits Aug 27, 2019
@kyliau
kyliau approved these changes Aug 27, 2019
@josephperrott
Copy link
Member

left a comment

LGTM

@mhevery mhevery closed this in 46caf88 Aug 29, 2019

sabeersulaiman added a commit to sabeersulaiman/angular that referenced this pull request Sep 6, 2019
refactor(language-service): add generic decorator property verificati…
…ons (angular#32252)

This PR makes finding class declarations properties in decorators are
applied to more generic to all properties that may be in a decorator,
and adds helper methods enabling getting the property assignment of a
property value and verifying that a property assignment is actually in a
decorator applied to a class.

This is done so that it will be easier to provide Angular definitions
for decorator properties moving forward. Most immediately, this will
provide decorator class verification for angular#32238.

PR Close angular#32252
sabeersulaiman added a commit to sabeersulaiman/angular that referenced this pull request Sep 6, 2019
feat(language-service): add definitions for templateUrl (angular#32238)
Adds support for `getDefinitionAt` when called on a templateUrl
property assignment.

The currrent architecture for getting definitions is designed to be
called on templates, so we have to introduce a new
`getTsDefinitionAndBoundSpan` method to get Angular-specific definitions
in TypeScript files and pass a `readTemplate` closure that will read the
contents of a template using `TypeScriptServiceHost#getTemplates`. We
can probably go in and make this nicer in a future PR, though I'm not
sure what the best architecture should be yet.

Part of angular/vscode-ng-language-service#111

PR Close angular#32238
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.