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): Make Definition and QuickInfo compatible with TS LS #31972

Closed
wants to merge 1 commit into from

Conversation

@kyliau
Copy link
Member

commented Aug 2, 2019

Now that the Angular LS is a proper tsserver plugin, it does not make
sense for it to maintain its own language service API.

This is part one of the effort to remove our custom LanguageService
interface.
This interface is cumbersome because it forces us to do two transformations:
ng def -> ts def -> lsp definition

The TS LS interface is more comprehensive anyway, so this allows the Angular LS
to return more information.

As part of this change, the hover text for an item is changed from
property propY of ClassX
to
(property) ClassX.propY
so that it's more consistent with tsserver result.

See screenshot below:
hover

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: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

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

@ngbot ngbot bot added this to the needsTriage milestone Aug 2, 2019

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

@kyliau kyliau force-pushed the kyliau:quick_info_and_def branch from 70a3514 to 0f0d2e4 Aug 2, 2019

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

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

packages/language-service/src/definitions.ts Outdated Show resolved Hide resolved
packages/language-service/src/hover.ts Show resolved Hide resolved
packages/language-service/src/hover.ts Show resolved Hide resolved
packages/language-service/src/hover.ts Outdated Show resolved Hide resolved
packages/language-service/src/ts_plugin.ts Outdated Show resolved Hide resolved
packages/language-service/src/types.ts Outdated Show resolved Hide resolved
packages/language-service/test/definitions_spec.ts Outdated Show resolved Hide resolved

@kyliau kyliau force-pushed the kyliau:quick_info_and_def branch 4 times, most recently from 33461ed to 2aa049f Aug 5, 2019

@kyliau

This comment has been minimized.

Copy link
Member Author

commented Aug 5, 2019

@ayazhafiz, ready for round 2 review. Mostly revamped definitions_spec.ts. PTAL, thank you.

packages/language-service/src/hover.ts Outdated Show resolved Hide resolved
fix(language-service): Make Definition and QuickInfo compatible with …
…TS LS

Now that the Angular LS is a proper tsserver plugin, it does not make
sense for it to maintain its own language service API.

This is part one of the effort to remove our custom LanguageService
interface.
This interface is cumbersome because we have to do two transformations:
  ng def -> ts def -> lsp definition

The TS LS interface is more comprehensive, so this allows the Angular LS
to return more information.

@kyliau kyliau force-pushed the kyliau:quick_info_and_def branch from 2aa049f to 6a41801 Aug 5, 2019

@alxhub alxhub closed this in a8e2ee1 Aug 8, 2019

kyliau added a commit to kyliau/vscode-ng-language-service that referenced this pull request Aug 8, 2019

feat: Backwards compat with new TS LS interface
angular/angular#31972 changes the Angular LS
interface to return TS data types. This PR makes the extension
compatible with both the old and new interface.

kyliau added a commit to angular/vscode-ng-language-service that referenced this pull request Aug 8, 2019

feat: Backwards compat with new TS LS interface
angular/angular#31972 changes the Angular LS
interface to return TS data types. This PR makes the extension
compatible with both the old and new interface.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.