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): TS references from template items #37437

Closed
wants to merge 3 commits into from

Conversation

ayazhafiz
Copy link
Member

Keen and I were talking about what it would take to support getting
references at a position in the current language service, since it's
unclear when more investment in the Ivy LS will be available. Getting TS
references from a template is trivial -- we simply need to get the
definition of a symbol, which is already handled by the language
service, and ask the TS language service to give us the references for
that definition.

This doesn't handle references in templates, but that could be done in a
subsequent pass.

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

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Feature

Does this PR introduce a breaking change?

  • Yes
  • No

@pullapprove pullapprove bot requested review from devversion and kyliau June 4, 2020 13:45
@ayazhafiz ayazhafiz removed the request for review from devversion June 4, 2020 13:45
@pullapprove pullapprove bot requested a review from devversion June 4, 2020 13:45
Copy link
Contributor

@kyliau kyliau left a comment

Choose a reason for hiding this comment

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

Did you mean to not expose this for now?

We'll need to make getReferencesAtPosition part of the plugin for it to be externally available.

const proxy: tss.LanguageService = Object.assign(
// First clone the original TS language service
{}, tsLS,
// Then override the methods supported by Angular language service
{
getCompletionsAtPosition,
getQuickInfoAtPosition,
getSemanticDiagnostics,
getDefinitionAtPosition,
getDefinitionAndBoundSpan,
});
return proxy;

packages/language-service/src/typescript_host.ts Outdated Show resolved Hide resolved
packages/language-service/test/references_spec.ts Outdated Show resolved Hide resolved
packages/language-service/test/references_spec.ts Outdated Show resolved Hide resolved
Copy link
Member

@devversion devversion left a comment

Choose a reason for hiding this comment

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

LGTM for bazel build change! (as always 😄 )

@ayazhafiz
Copy link
Member Author

Did you mean to not expose this for now?

We'll need to make getReferencesAtPosition part of the plugin for it to be externally available.

I think it's okay to expose it in this package, it doesn't need to be known by the VSCode extension until it's in a more useful state IMO.

@ayazhafiz ayazhafiz requested a review from kyliau June 5, 2020 14:23
Keen and I were talking about what it would take to support getting
references at a position in the current language service, since it's
unclear when more investment in the Ivy LS will be available. Getting TS
references from a template is trivial -- we simply need to get the
definition of a symbol, which is already handled by the language
service, and ask the TS language service to give us the references for
that definition.

This doesn't handle references in templates, but that could be done in a
subsequent pass.

Part of angular/vscode-ng-language-service#29
});
});

// TODO: override parsing-cases#TemplateReference for inline templates.
Copy link
Contributor

Choose a reason for hiding this comment

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

This'd be nice, TemplateReference already has an external template though..

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah but we can just overwrite and replace templateUrl

packages/language-service/test/references_spec.ts Outdated Show resolved Hide resolved
packages/language-service/test/references_spec.ts Outdated Show resolved Hide resolved
@kyliau kyliau added area: language-service Issues related to Angular's VS Code language service action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews target: patch This PR is targeted for the next patch release labels Jun 8, 2020
@ngbot ngbot bot modified the milestone: needsTriage Jun 8, 2020
@ayazhafiz ayazhafiz added the action: merge The PR is ready for merge by the caretaker label Jun 8, 2020
@kyliau kyliau removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Jun 8, 2020
atscott pushed a commit that referenced this pull request Jun 9, 2020
Keen and I were talking about what it would take to support getting
references at a position in the current language service, since it's
unclear when more investment in the Ivy LS will be available. Getting TS
references from a template is trivial -- we simply need to get the
definition of a symbol, which is already handled by the language
service, and ask the TS language service to give us the references for
that definition.

This doesn't handle references in templates, but that could be done in a
subsequent pass.

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

PR Close #37437
@atscott atscott closed this in 55979fe Jun 9, 2020
ngwattcos pushed a commit to ngwattcos/angular that referenced this pull request Jun 25, 2020
)

Keen and I were talking about what it would take to support getting
references at a position in the current language service, since it's
unclear when more investment in the Ivy LS will be available. Getting TS
references from a template is trivial -- we simply need to get the
definition of a symbol, which is already handled by the language
service, and ask the TS language service to give us the references for
that definition.

This doesn't handle references in templates, but that could be done in a
subsequent pass.

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

PR Close angular#37437
@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 Jul 10, 2020
profanis pushed a commit to profanis/angular that referenced this pull request Sep 5, 2020
)

Keen and I were talking about what it would take to support getting
references at a position in the current language service, since it's
unclear when more investment in the Ivy LS will be available. Getting TS
references from a template is trivial -- we simply need to get the
definition of a symbol, which is already handled by the language
service, and ask the TS language service to give us the references for
that definition.

This doesn't handle references in templates, but that could be done in a
subsequent pass.

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

PR Close angular#37437
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: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants