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): include compilerOptions.rootDir in rootDirs #40243

Closed
wants to merge 1 commit into from

Conversation

kyliau
Copy link
Contributor

@kyliau kyliau commented Dec 22, 2020

When resolving references, the Ivy compiler has a few strategies it could use.

For relative path, one of strategies is RelativePathStrategy. This strategy
relies on compilerOptions.rootDir and compilerOptions.rootDirs to perform
the resolution, but language service only passes rootDirs to the compiler,
and not rootDir.

In reality, rootDir is very different from rootDirs even though they
sound the same.
According to the official TS documentation,

rootDir specifies the root directory of input files. Only use to control
the output directory structure with --outDir.

rootDirs is a list of root folders whose combined content represent the
structure of the project at runtime. See Module Resolution documentation
for more details.

For now, we keep the behavior between compiler and language service consistent,
but we will revisit the notion of rootDir and how it is used later.

Fix angular/vscode-ng-language-service#1039

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 added area: language-service Issues related to Angular's VS Code language service target: patch This PR is targeted for the next patch release area: compiler Issues related to `ngc`, Angular's template compiler labels Dec 22, 2020
@kyliau kyliau requested a review from atscott December 22, 2020 23:21
@ngbot ngbot bot added this to the Backlog milestone Dec 22, 2020
@google-cla google-cla bot added the cla: yes label Dec 22, 2020
@kyliau kyliau force-pushed the tsconfig-root-dir branch 4 times, most recently from 750eff0 to 5b6f2da Compare December 23, 2020 04:50
@pullapprove pullapprove bot requested a review from IgorMinar December 23, 2020 04:50
@pullapprove pullapprove bot added the area: dev-infra Issues related to Angular's own dev infra (build, test, CI, releasing) label Dec 23, 2020
@kyliau kyliau removed the request for review from IgorMinar December 23, 2020 04:55
@pullapprove pullapprove bot requested a review from IgorMinar December 23, 2020 04:55
@kyliau kyliau requested review from josephperrott and removed request for IgorMinar December 23, 2020 04:56
When resolving references, the Ivy compiler has a few strategies it could use.

For relative path, one of strategies is [`RelativePathStrategy`](
https://github.com/angular/angular/blob/master/packages/compiler-cli/src/
ngtsc/imports/README.md#relativepathstrategy). This strategy
relies on `compilerOptions.rootDir` and `compilerOptions.rootDirs` to perform
the resolution, but language service only passes `rootDirs` to the compiler,
and not `rootDir`.

In reality, `rootDir` is very different from `rootDirs` even though they
sound the same.
According to the official [TS documentation][1],
> `rootDir` specifies the root directory of input files. Only use to control
> the output directory structure with --outDir.

> `rootDirs` is a list of root folders whose combined content represent the
> structure of the project at runtime. See [Module Resolution documentation](
> https://www.typescriptlang.org/docs/handbook/
> module-resolution.html#virtual-directories-with-rootdirs)
> for more details.

For now, we keep the behavior between compiler and language service consistent,
but we will revisit the notion of `rootDir` and how it is used later.

Fix angular/vscode-ng-language-service#1039

[1]: https://www.typescriptlang.org/docs/handbook/compiler-options.html
Copy link
Member

@josephperrott josephperrott left a comment

Choose a reason for hiding this comment

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

LGTM

@alyst
Copy link

alyst commented Jan 6, 2021

Would be really nice to include this PR in the next patch release of NG11 as currently the language service is mostly broken for my multiproject setup. I'm a bit scared to continue like that for one more week :)

@AndrewKushnir AndrewKushnir removed the request for review from alxhub January 6, 2021 18:13
@kyliau kyliau added action: merge The PR is ready for merge by the caretaker target: minor This PR is targeted for the next minor release and removed target: patch This PR is targeted for the next patch release labels Jan 6, 2021
@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 Feb 6, 2021
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: compiler Issues related to `ngc`, Angular's template compiler area: dev-infra Issues related to Angular's own dev infra (build, test, CI, releasing) area: language-service Issues related to Angular's VS Code language service cla: yes target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ngcc crash with monorepo / multi-project config - Ivy Native LS
5 participants