Skip to content

Watch for the creation of missing files#16527

Closed
amcasey wants to merge 11 commits into
microsoft:masterfrom
amcasey:PrelimVsts434619
Closed

Watch for the creation of missing files#16527
amcasey wants to merge 11 commits into
microsoft:masterfrom
amcasey:PrelimVsts434619

Conversation

@amcasey
Copy link
Copy Markdown
Member

@amcasey amcasey commented Jun 14, 2017

When we discover that a file is missing, call sys.watchFile on its path so that we can update our state when it is created.

  1. For tsc --watch, we schedule a new build
  2. For tsserver, we update the containing projects

Caveat: VS will not know about the state change in tsserver until the next time it synchronizes its project list

amcasey added 2 commits June 14, 2017 10:44
When we discover that a file is missing, call sys.watchFile on its path
so that we can update our state when it is created.

1) For tsc --watch, we schedule a new build
2) For tsserver, we update the containing projects

Caveat: VS will not know about the state change in tsserver until the
next time it synchronizes its project list
@amcasey amcasey requested review from a user, billti, mhegazy and rbuckton June 14, 2017 17:47
@msftclas
Copy link
Copy Markdown

@amcasey,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot

@amcasey
Copy link
Copy Markdown
Member Author

amcasey commented Jun 14, 2017

TODO: I'm still adding tests and cleaning things up, but I'd appreciate feedback on the general approach

@amcasey
Copy link
Copy Markdown
Member Author

amcasey commented Jun 14, 2017

As far as I know, I've addressed all of @mhegazy's offline feedback. I'll focus on testing until I hear otherwise.

@amcasey
Copy link
Copy Markdown
Member Author

amcasey commented Jun 15, 2017

Added structure reuse tests and fixed bug where reused program structure dropped missing file list.

Comment thread src/compiler/program.ts
filesByName.set(filePaths[i], newSourceFiles[i]);
}

for (const p of oldProgram.getMissingFilePaths()) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this needs to be under the check of

if (oldProgram.getMissingFilePaths) {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oops :)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Under what circumstances will getMissingFilePaths ever be undefined?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I made the property optional in case there were existing implementers of the interface. Was that over-cautious?

Comment thread src/compiler/program.ts
filesByName.set(filePaths[i], newSourceFiles[i]);
}

for (const p of oldProgram.getMissingFilePaths()) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Under what circumstances will getMissingFilePaths ever be undefined?

Comment thread src/server/server.ts
return;
}

// removed = deleted ? true : (added ? false : undefined)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can this comment be removed?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It was intended as an explanation. If you don't believe it's helpful, I'm happy to remove it.

@amcasey
Copy link
Copy Markdown
Member Author

amcasey commented Jun 21, 2017

I've posted a cleaned up PR: #16684.
@sheetalkamat: I made the change you suggested.
@rbuckton: I've responded to your questions above but not yet made any code changes.

@amcasey amcasey closed this Jun 21, 2017
@amcasey amcasey deleted the PrelimVsts434619 branch September 7, 2017 23:35
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants