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

tsserver should provide an efficient way to compute project errors #9125

Closed
dbaeumer opened this issue Jun 13, 2016 · 8 comments
Closed

tsserver should provide an efficient way to compute project errors #9125

dbaeumer opened this issue Jun 13, 2016 · 8 comments

Comments

@dbaeumer
Copy link
Member

dbaeumer commented Jun 13, 2016

TypeScript Version:

nightly (1.9.0-dev.20160217)

VS Code will soon provide an error list. This made it obvious that the current behavior of validating the open files only is very dissatisfying as usually more errors come if more files get opened.

I started to experiment with the 'geterrForProject' route the tsserver provides. However the implementation doesn't scale well. Using the route on a VS Code workspace with a delay between file checking of 0 takes 4 minutes to compute all project errors. I therefor propose to introduce the concept of builders into the tsserver to more efficiently check for errors depending on the module system and the project type. The following builder would be present:

NullBuilder

If the editor doesn't want project building and always requests errors by itself.

VirtualProjectBuilder

Builder used if no tsconfig file is present. Since we can't assume a module system we have to validate all files in the virtual project. However we should allow configuring a maximum of validated files and should show a warning if that maximum is exceed. In addition we can optimize error checking by keeping a signature for every files in the virtual project. The signature is a hash (md5) over the declaration file of a ts file. If a file changes but is signature doesn't (e.g. its public shape stayed the same) there is no need to recheck other files.

InternalProjectBuilder

This is a builder for TS project configured for the internal / none module system. It will function them same way as the VirtualProjectBuilder.

ModuleProjectBuilder

We have a configured module system and therefore clear dependencies between the TS files. The builder will work as follows:

  • maintains a reverse dependency graph
  • if a file changes only the files that depend on the changed files will be rechecked
  • as with the other builders it uses a signature computed on the public shaped to stop checking if the shape stays the same.
  • for optimal performance the builder should be able to persist some state to avoid unnecessary re-computation of errors on next startup.

Know open issues:

  • currently the language service API only allows to emit all files (js, declaration and map). To optimize the implementation it would be necessary to emit declaration files only.
  • How should we let the tsserver know whether it should install a builder or use the null builder. Could be a command line option passed the tsserver (since it is editor specific) or we could extend the configure event.
  • Should we allow to control this per project as well via a property in the tsconfig.json (comparable to compileOnSave)

I have started an implementation of those builders which I will soon make available on a branch in the repository. Would be great to get feedback on whether the TS team thinks this could be an reasonable approach.

Credits: gulp-tsb and the Eclipse Java compiler use a comparable approach to speed up error checking and compilation.

@dbaeumer
Copy link
Member Author

@dbaeumer
Copy link
Member Author

I have a question as well. I noticed that externalModuleIndicator is undefined in files like this:

declare module "abc" {
export function foo(): void;
}

I thought this style is used to type external modules written in JS. Is there a reason why externalModuleIndicator is undefined?

@angelozerr
Copy link

@dbaeumer I tell me if we could use your work about builder to extend it and support tslint too? It should very cool if we could have an extension with your builder to implement it like we wish like with tslint.

What do you think about that?

@dbaeumer
Copy link
Member Author

@angelozerr to my knowledge tslint doesn't check any inter file dependencies. Assuming this is correct then a builder for tslint will be very simple. Validate all files and then only validate the changed files. However we could share some implementation techniques to keep a state of file being validated between sessions. But I haven't settle on an optimal implementation here. Will share as soon as I have something implemented.

The builder for TS is quite more complicated if you use a module system since if module A referenced module B then a change in B need to recompile A even if A isn't open.

@dbaeumer
Copy link
Member Author

And another questions. Conisder the following example:

file someModule.d.ts

declare module "someModule" {
   ....
}

file: use.ts

import * as sm from "someModule"

sm.foo();

The sourceFile for use.ts has an resolvedModule property with an key "someModule" however the value of the key is undefined. Is this on purpose. Would be very helpful if the value would point to something that points to the someModule.d.ts file.

@zhengbli @vladima can you help me with that. I am a little stuck here. Thanks!

@mhegazy
Copy link
Contributor

mhegazy commented Jun 14, 2016

@dbaeumer there are two steps to resolve these references, once during building the program. these try to resolve a module import to a file. if that fails, nothing is written to the entry. later on, in the checker, when resolving the import, if a file is not found, we look in the global scope, to see if an ambient declaration exist. this can be not be written to the file, as the same SourceFile can be shared between multiple Programs.

@zhengbli
Copy link
Contributor

The idea and model is quite neat. Regarding the per-project setting, I don't see much value in it though. It requires the user to know quite some details about the implementation of the tooling, which can be confusing and wouldn't result in much differences in the user's perspective. In addition, editors may have different preferences because the way it reports diagnostics can be different; so it may be more about per-user instead of per-project. Besides in the case with no config files, it's hard for the user to control.

The editor inevitably needs to tell the server its capability / preferences for builds in some way, therefore it seems an extra step to carry information in tsconfig, if there are no real need for the options to be customized per project.

@mhegazy
Copy link
Contributor

mhegazy commented Sep 29, 2016

this should be addressed by #11229

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants