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

refactoring(language-service): Remove 'context' used for module resolution #32015

Closed
wants to merge 1 commit into from

Conversation

@kyliau
Copy link
Member

commented Aug 6, 2019

The language service relies on a "context" file that is used as the
canonical "containing file" when performing module resolution.
This file is unnecessary since the language service host's current
directory always default to the location of tsconfig.json for the
project, which would give the correct result.

This refactoring allows us to simplify the "typescript host" and also
removes the need for custom logic to find tsconfig.json.

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 6, 2019

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

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

@kyliau kyliau force-pushed the kyliau:context branch from 99ebcbf to 46efb39 Aug 6, 2019

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

@kyliau kyliau force-pushed the kyliau:context branch from 46efb39 to fe04910 Aug 6, 2019

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

@ayazhafiz

This comment has been minimized.

Copy link
Contributor

commented Aug 6, 2019

The language service relies on a "context" file that is used as the
canonical "containing file" when performing module resolution.

Wow, was this ever needed previously?

@kyliau kyliau requested a review from alxhub Aug 6, 2019

@kyliau

This comment has been minimized.

Copy link
Member Author

commented Aug 6, 2019

The language service relies on a "context" file that is used as the
canonical "containing file" when performing module resolution.

Wow, was this ever needed previously?

Yeah, I'm not sure why. The (now deleted) comment says that server will always return empty string for getCurrentDirectory, but that's not the case now. Maybe it was the old behavior.

fix(language-service): Remove 'context' used for module resolution
The language service relies on a "context" file that is used as the
canonical "containing file" when performing module resolution.
This file is unnecessary since the language service host's current
directory always default to the location of tsconfig.json for the
project, which would give the correct result.

This refactoring allows us to simplify the "typescript host" and also
removes the need for custom logic to find tsconfig.json.

@kyliau kyliau force-pushed the kyliau:context branch from fe04910 to d8117b4 Aug 8, 2019

@alxhub
alxhub approved these changes Aug 13, 2019

@kara kara closed this in a91ab15 Aug 13, 2019

sabeersulaiman added a commit to sabeersulaiman/angular that referenced this pull request Sep 6, 2019
fix(language-service): Remove 'context' used for module resolution (a…
…ngular#32015)

The language service relies on a "context" file that is used as the
canonical "containing file" when performing module resolution.
This file is unnecessary since the language service host's current
directory always default to the location of tsconfig.json for the
project, which would give the correct result.

This refactoring allows us to simplify the "typescript host" and also
removes the need for custom logic to find tsconfig.json.

PR Close angular#32015
@angular-automatic-lock-bot

This comment has been minimized.

Copy link

commented Sep 15, 2019

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 Sep 15, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants
You can’t perform that action at this time.