Prototype TS Compile on Save

[mjbvz]

Initial prototype enabling compileOnSave for TypeScript in VSCode

TODO:

• Possibly gate using an experimental setting. We probably want to wait for TS 2.5 for this as well.
• Better handle reading compileOnSave from project config file. Possibly could benefit from Expose ContextEvent in TS Server Protocol TypeScript#17659
• Request errors for compiled files. Would benefit from Feedback to improve compileOnSave with tsserver. TypeScript#17630
• Investigate why node_modules files are sometimes included in affectedFileList
• Check perf implications on larger projects

@dbaeumer Can you please remind me what the main concerns were for enabling compileOnSave?

// cc @sheetalkamat

[mention-bot]

@mjbvz, thanks for your PR! By analyzing the history of the files in this pull request, we identified @dbaeumer and @egamma to be potential reviewers.

[dbaeumer]

When I last looked into this I wanted to implement build On Save which might be different than what you now try to do with compile on save. Compile on save emits all necessary JS files after one file changes. But it doesn't allow you to get all TS errors when a project opens (like we do in VS Code with npm run watch). Problems I saw / addressed:

• how to create a first set of all emitted JS files with the approach. And how to produce a set of project errors when a TS project opens. The problem now is that if a file changes we emit problems for all effected files but not on initial startup. That might be very confusing for users. I had a builder state which I persisted between tsserver sessions.
• performance since we need to request emit for every single file. Same for checking. Looks like for every file we sent 3 requests to the server
• queuing and updating. Consider a user changes file A which results in 20 files to be recompiled. You get them and then he changes file B which results in 30 files to be recompiled which overlap with the 20. I implemented a queue where I managed the files and ensured that files only get compiled once.
• always compile and emit the file the user types in first.

For sure more if I continue thinking about it. In general I believe that having something like a builder can only be implemented inside the TSServer in an efficient way.

[sheetalkamat]

@mjbvz @dbaeumer With microsoft/TypeScript#17669 we will have implementation in builder to be able to give diagnostics/cache them based on the changes. It would be possible to give the error list as well as the emit file list through that. With https://github.com/Microsoft/TypeScript/pull/17269/files#diff-efa731ca7c6279501f903db87403d2beR627 I already have todo that I need to address before merging about the event we send so that event handler receives the event about change in project structure to be able to update the error checks.
This is what I am thinking you need:

• We need to send event whenever project structure changes. This way you don't need to watch the files (closed files or config file) to decide when to update the error list on that event.
• Currently ContextEvent is sent when some file is deleted: https://github.com/Microsoft/TypeScript/blob/f124e199718b9cda06654af2ce487f1d6a332b34/src/server/editorServices.ts#L606 and that then updates the errors for only open files. https://github.com/Microsoft/TypeScript/blob/f124e199718b9cda06654af2ce487f1d6a332b34/src/server/session.ts#L333
• I think as a start we could use the same event for not just file change but project structure change?

Apart from this I think these are the few things you would need:

• Event that is sent whenever (closed) file is updated? (note we do not watch open files since those updates are through editors and apis that let us know changes.)
• Event that is sent whenever project structure is changed?
• Capability to save file instead of just getting affected file list and then doing actual emit request

Also you could use geterrForProject (to get errors for complete project instead of just specific file https://github.com/Microsoft/TypeScript/blob/master/src/server/session.ts#L1598) or keep the same implementation and request errors for specific files. And was there reason to get semantic diagnostics before the syntactic errors?

[dbaeumer]

@sheetalkamat thanks for the update. When I implemented the builder inside the tsserver I tried to use geterrForProject however this didn't scale well for large projects. When I tried this with a VS Code workspace it took minutes for the call to complete. This is why I implemented the builder state. With that I was able to open a project and reconstruct the error state in ~10 seconds for VS Code.

This code still exists here: https://github.com/Microsoft/TypeScript/tree/dbaeumer/9125. Vlad took parts of the code and moved it into the current builder of the tsserver. But may be we can reuse more of it. For example here is the state handling https://github.com/Microsoft/TypeScript/blob/dbaeumer/9125/src/server/builder.ts#L1400

[sheetalkamat]

@mjbvz can you check if microsoft/TypeScript#17820 seems like what you could use to rely on us to notify the changes to project? Currently i replaced this for the internal context event which was sent when file was deleted and it would update the error checks for all the open files. Instead I am now scheduling error checks for the files that are open but have changed since last update event (host should get semanticDiag and syntaxDiag events for those files) Also i created new event: "projectChanged" where you can get the list of files to emit and changed files. The filesToEmit are the files on which you want to call compileOnSaveEmitFile and that would be sufficient to have the correct result. It also has this changedFiles, in case you prefer scheduling those diagnostic checks. Note that this event occurs when there is delay in the project structure update (eg. if the closed file/config file changes/there are changes applied to already open files)

[mjbvz]

@sheetalkamat Thanks. So with this API, we'd only have to listen to ProjectChanged events, and then call compileOnSaveEmitFile for the files that need to be recompiled, correct? All the batching and other builder logic would be handled on the TS side?

[sheetalkamat]

@mjbvz Yes. You wont also have to watch any of the closed script infos or config file. Note that we still rely on editor providing reload/edits for the open script infos.

[dbaeumer]

@mjbvz should we close this or is it still something you are working on?

[alexr00]

Closing due to lack of activity. We can reopen if this is incorrect.