Skip to content

Compile ts files with tsserver #109

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

Closed
angelozerr opened this issue Nov 7, 2016 · 21 comments
Closed

Compile ts files with tsserver #109

angelozerr opened this issue Nov 7, 2016 · 21 comments
Milestone

Comments

@angelozerr
Copy link
Owner

TypeScript 2.1.0 will provide new command to compile TypeScript files which will improve a lot performances (I think).

@angelozerr
Copy link
Owner Author

@nimo23 I have started to implement compileOnSave with tsserver. The "compileOnSaveEmitFile" is available since TypeScript 2.0.5, so if you install typescript.java 1.2.0 wich provides TypeScript 2.0.9 as embed TypeScript, you can play with this feature.

Once job https://opensagres.ci.cloudbees.com/job/angular2-eclipse/92/ will be finished you will able to install Angular2 Eclipse 1.2.0-SNAPSHOT with update site https://github.com/angelozerr/angular2-eclipse/wiki/Installation-Update-Site

Please note that my work is not finished:

  • buildOnSave doens't work now (I must fix that)
  • error is not displayed in the Problem View (I must fix that)
  • it cannot refresh the js, map file generated for the first time, because tsserver compile is done with async mean to speed the compilation an dnot freeze the builder (I think it will be the restriction).

I have enable compileOnSave with tsserver when TypeScript 2.0.5 is consumed. Do you think we need a preferences to use tsc and not tsserver when TypeScript 2.0.5 is installed?

@nimo23
Copy link

nimo23 commented Nov 13, 2016

Hello, thank you!

Do you think we need a preferences to use tsc and not tsserver when TypeScript 2.0.5 is installed?

I think, soon all users will at least have ts version 2.1, so tsserver can be used. There is no need to give user the option to use tsc instead of tsserver as TS IDE will include ts version 2.1 soon (it is on the way: https://blogs.msdn.microsoft.com/typescript/2016/11/08/typescript-2-1-rc-better-inference-async-functions-and-more/).

In future, are there any benefits or reasons to use tsc instead of tsserver? If not, then users need no preference to choose between tsc and tsserver.

@angelozerr
Copy link
Owner Author

In future, are there any benefits or reasons to use tsc instead of tsserver? If not, then users need no preference to choose between tsc and tsserver.

The big problem is that I cannot refresh correctly the emitted files. See microsoft/TypeScript#12124, with tsc I can because it returns the emited files.

@nimo23
Copy link

nimo23 commented Nov 13, 2016

I understand. I hope microsoft/TypeScript#12124 will be done.

As you said:

In Eclipse, when a file is created outside Eclipse (like tsserver generates js, map files), the files doesn't appear in the Navigator files of the Eclipse. We must do a refresh at hand.

Maybe this is related to https://bugs.eclipse.org/bugs/show_bug.cgi?id=507280 ?

I dont know, however, would it be a problem to do always (as long as this issue is not resolved) a full refresh of the actual TypeScript Build Path when compiling is done.

"Async compile" -> "Callback/promise" -> "Refresh full build path"?

@angelozerr
Copy link
Owner Author

to do always (as long as this issue is not resolved) a full refresh

I do a refresh, but the problem is that the refresh of js, map file is done BEFORE the emited file is generated. CompileOnSaveEmitFile command is asynch, so it generates the js, map files but don't wait for that thoses files are finished to generate. I think I will keep this problem for the moment.

@nimo23
Copy link

nimo23 commented Nov 13, 2016

I dont know about CompileOnSaveEmitFile command. Normally, async methodes should have callbacks/promises so you can hook into it when command is done. So when async method is done, "callback for refresh" is called AFTER the emited file is generated. However, I dont know if this method supports that.

@jcompagner
Copy link

i guess we could just depend on General->Workspace->refresh using native hooks or polling to let eclipse auto refresh the generated js files?

@angelozerr
Copy link
Owner Author

i guess we could just depend on General->Workspace->refresh

I don't know how it works. Is this feature is able to track a file creation which is not created with Eclipse?

@jcompagner
Copy link

yes, you can quickly see it working for you self

open up in the navigator a project and look at a certain folder or the root of that project, go with the file explorer of windows and do new ->create text file
at the moment you save that you will also see eclipse updating it right away, if you then delete it will be gone, if you have that option enabled, if you do the same with that option disabled then you will not see those updates.

So i guess for now for people really wanting to have the latest js files directly updated they can enable that. But i don't know how that works for other OS's. Because i think its really a native thing (maybe for other besides windows they do pure polling) in eclipse i think its this native pluging: org.eclipse.core.resources.win32.x86_64_3.5.0.v20140124-1940.jar that does that job

@nimo23
Copy link

nimo23 commented Dec 7, 2016

open up in the navigator a project and look at a certain folder or the root of that project, go with the file explorer of windows and do new ->create text file

this scenario works well when using eclipse "refresh using native hooks", however, it does NOT work when file is created by terminal or the like. For example, when using angular cli, files are created by terminal commands and eclipse "refresh using native hooks" does not work anymore. I reported this issue in https://bugs.eclipse.org/bugs/show_bug.cgi?id=507280.

You can easily see that, when using eclipse built-in-terminal and do something like "touch test.ts" within your project root. Unfortunatly, Eclipse does not refresh and show this newly created file even when "refresh using native hooks" is enabled.

@jcompagner
Copy link

i guess the mac (thats where you test against?) doesn't have that native code? But then i think it should fall back into polling
but as an example under windows the "touch" variant in the terminal of eclipse works fine:

echo $null >test.txt

it popups right away.

@jcompagner
Copy link

the thing is that in the end it would be nice that we wouldn't depend on refresh stuff like this.
But for this plugin we can't do it anyway else.

I had for exampe big problems with that setting, because i have a project that is a maven java and typescript project within eclipse

so it has a maven builder, the java builder and the type script builder. If then the bin dir of the classes is also the default \target\classes of maven (where also the maven plugin works in) then the auto refresh really screws stuff up. Because the builder is constantly triggered and the maven plugin also works outside of eclipse (and doesn't do a refresh) so the classes that the java builder generates are constantly gone. This is somehow not really a problem when typescript is not added, but that one also changes stuff again and that triggers maven again to build, So i constantly was loosing my compiled classes.
(and because of that compile errors on many other projects)

If the native refresh was disabled it worked a bit better (because i think now files are not constantly coming in, its more 1 big refresh inside eclipse).
But having 2 builders (of the 3) that work "outside" of eclipse file api and those are depending on refreshes is quite a challenge and you really need to be carefull what folders (in and output) they share.

@nimo23
Copy link

nimo23 commented Dec 8, 2016

i guess the mac (thats where you test against?)

yes, on mac.

But having 2 builders (of the 3) that work "outside" of eclipse file api and those are depending on refreshes is quite a challenge and you really need to be carefull what folders (in and output) they share.

So maybe such cases should be reported on https://bugs.eclipse.org/bugs.

@angelozerr
Copy link
Owner Author

@nimo23 @jcompagner to fix problem with refresh of emited files, tsserver should support that. See microsoft/TypeScript#12124

Hope TypeScript team will implement this feature.

@angelozerr
Copy link
Owner Author

@lorenzodallavecchia I have implemented compile on save with tsserver. Please give me feedback and PR if you think you could improve it. Thanks.

The pros are:

  • compile on save with tsserver is activated if you are using TypeScript >=2.0.5
  • compile on save with tsserver is very quick (compare to tsc)
  • compile on save is able to manage file dependencies

The cons are:

  • I cannot refresh emited files
  • I cannot manage custom errors (like you have do with your PR)

@lorenzodallavecchia
Copy link
Contributor

Terrific news @angelozerr!
I will surely try it in the coming days and let you know.

I'm glad you are mentioning file dependencies as one of the pros: in fact I had noted recently that the tsc-based is not capable of recompiling even trivial dependencies within the same project.

Too bad about the errors. Provided the time, I will also try to look into that.

In the meantime, thanks!

@lorenzodallavecchia
Copy link
Contributor

lorenzodallavecchia commented Feb 6, 2017

Ok @angelozerr I have done some testing of the new tsserver-based CoS and now I understand a lot of things better. The following is a list of findings and problems.

Language services require at least one open file

This one somehow shocked me: for tsserver to "know" about a tsconfig.json and all of its files, at least one file must be open (that means, an open command sent to the server and still no close command).

If all files are closed, tsserver will not respond to any query about the project (not compileOnSaveAffectedFileList, not even projectInfo).
For example, if you

  1. close all files,
  2. modify a .ts file out of Eclipse,
  3. refresh the file in Eclipse,

the file itself is not recompiled!

This can cause all sorts of problems in Eclipse: for example when refreshing external changes, doing a Find-Replace, moving/renaming files or checking out from Git.

Multiple tsconfig.json support

The tsserver does support multiple tsconfig.json files. However, both tsconfigs must have at least one file open (because of the above problem). For example, in #128, the scenario with single-poject-multi-config would work perfectly provided that one file is opened from each of the tsconfig scopes.

Added/deleted files

When a file is removed in Eclipse, no notification is sent to tsserver.
When a file is added in Eclipse, the client is asking for compileOnSaveAffectedFileList, but the server is responding that no files are affected. I think this is caused by the fact that the server was not informed about any actual "change", so it considers the new file unchanged.

The overall supporto for project-level changes seems completely lacking from tsserver: this is probably a result of it being used as backend for tools like VSCode that are still not providing the level of project scope you can see in Eclipse. For example, it is still normal in VSCode to not expect error to be listed for closed files.

Unfortunately, for CoS to be realiable, each delta in the Eclipse workspace must be communicated to tsserver. The client cannot risk to have its incremental state misaligned with the server's.
If there is no way of overcoming this in tsserver (is there, Angelo?) the only option I see is to force a compileOnSave of all files when a delta cannot be sent to the server. The price would be worse performance for some operations, but at least the developer could rely on the IDE.

Multiple Eclipse projects

The situation of ts files referencing others in different Eclipse projects is a different story. In this case, tsserver alone cannot solve all issues for the simple fact that typescript.java is opening one tsserver for each Eclipse project. Of course this is inevitable, since the TS version to use is configured ad project level.

In this case the only solution is that the Eclipse builder must be made multi-project aware, so that a project can also be informed about the deltas of other projects that it depends on.
This is feasible (I have even half-implemented something), but not really clean because I must hard-code some of the tsconfig logic in Eclipse.

There are also other issues caused by not knowing in tsserver which paths may contain a future imported file, and so not knowing in advance which projects are needed for building another. This could be fixed by requiring the user to set Project References explicitly.

Considerations

I think the above problems are mostly caused by tsserver shortcomings, but in my opinion they need to be addressed for the CoS feature to be realied upon.

All in all, tsserver seems to lack good support beyond the scope of single open files. This is bad for Eclipse, which is a workspace-based IDE.

This may change, of course, as TypeScript implements features like microsoft/TypeScript#3469.

@angelozerr
Copy link
Owner Author

For emitted file, we must wait for https://github.com/Microsoft/TypeScript/pull/13915/files

@angelozerr
Copy link
Owner Author

@lorenzodallavecchia many thanks for your great analyse! Could you create a new issue with your analyse please. I would like to close this issue.

I would like to use tsserver for compile on save because VSCode will use it and if typescript.java have some trouble with tsserver, VSCode will have too. I would like to avoid developping some extra feature although tsserver should support it.

@lorenzodallavecchia
Copy link
Contributor

lorenzodallavecchia commented Feb 9, 2017

Could you create a new issue with your analyse please. I would like to close this issue.

No problem. I will split them in two-three issues, mainly to decouple the "multiple project" problem from the others that may affect all users.

I agree that compiling with tsserver is the way to go: for the moment I am still using tsc for reliability over performance, but I would be more than happy to switch to tsserver.

@lorenzodallavecchia
Copy link
Contributor

I opened #142 #143 and #144 to track the found issues.

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

No branches or pull requests

4 participants