Skip to content

Fix detecting default project when file is part for more than one project but not part of default configured project (eg because its output of that projet) #38429

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

Merged
merged 1 commit into from
May 12, 2020

Conversation

sheetalkamat
Copy link
Member

Fixes #38366

…ject but not part of default configured project (eg because its output of that projet)

Fixes #38366
@sheetalkamat
Copy link
Member Author

@typescript-bot cherry pick this to release-3.9

@sheetalkamat
Copy link
Member Author

@typescript-bot cherry-pick this to release-3.9

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 8, 2020

Heya @sheetalkamat, I've started to run the task to cherry-pick this into release-3.9 on this PR at 6e3c1d1. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

Hey @sheetalkamat, I've opened #38431 for you.

typescript-bot pushed a commit to typescript-bot/TypeScript that referenced this pull request May 8, 2020
Component commits:
6e3c1d1 Fix detecting default project when file is part for more than one project but not part of default configured project (eg because its output of that projet) Fixes microsoft#38366
@@ -1743,7 +1743,9 @@ namespace ts.server {

return project?.isSolution() ?
project.getDefaultChildProjectFromSolution(info) :
project;
project && projectContainsInfoDirectly(project, info) ?
Copy link
Member

Choose a reason for hiding this comment

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

Do the other callers of getConfigFileNameForFile and findConfiguredProjectByProjectName need this check too?

Copy link
Member Author

Choose a reason for hiding this comment

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

No.. this is the change to determine default project for the file...

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure I follow. Why is direct containment more important for the default project than for other projects?

Copy link
Member Author

Choose a reason for hiding this comment

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

findConfiguredProjectByProjectName project finds the configured project by the given name and has nothing to do with scriptinfo.
Similary getConfigFileNameForFile finds config file name by traversing directories for tsconfig/jsconfig json files. Nothing else matters.

This function is what determines the default project for given script info and hence the additional logic

Copy link
Member

@amcasey amcasey left a comment

Choose a reason for hiding this comment

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

I don't understand why the change covers all the scenarios it needs to, but I believe it's correct in the scenario it does cover, so I'm fine with merging and discussing offline.

@sheetalkamat sheetalkamat merged commit 5d6a5d0 into master May 12, 2020
@sheetalkamat sheetalkamat deleted the defaultConfigProject branch May 12, 2020 17:17
sheetalkamat added a commit that referenced this pull request May 12, 2020
Component commits:
6e3c1d1 Fix detecting default project when file is part for more than one project but not part of default configured project (eg because its output of that projet) Fixes #38366

Co-authored-by: Sheetal Nandi <[email protected]>
cangSDARM added a commit to cangSDARM/TypeScript that referenced this pull request May 13, 2020
* upstream/master:
  Update user baselines
  Fix detecting default project when file is part for more than one project but not part of default configured project (eg because its output of that projet) (microsoft#38429)
  fix(37877): include in NavigationBar default exported child items (microsoft#38255)
  fix: add missing semi-colon to `__exportStar` unnamed function
  Update baselines.
  Add and use the 'intersperse' helper function.
  Don't add duplicates of JSDoc comments.
  Added tests for union types with identical doc comments.
  feat(38225): change diagnostic message for remove braces from arrow function body
  Add outlining spans for object destructuring elements
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Project does not contain document" Error Causes Types to Stop Resolving in VS Code
3 participants