Some improvements

[Phaiax]

trafficstars

Hey

I've worked a little bit on the code in the dev branch.

Former i had the problem, that there was a bunch of unexecuted showError commands in the async tss queue if I type a character just every debounce_delay seconds. Every showError takes 5-10 seconds on my project so it could easily take up to a minute until i could see the most recent errors. This took CPU time and slowed the builds also.

I did a lot, these are the main changes:

• still two instances of tss.js for each project (one for completions etc, one for the slow showErrors)
• but both of them are async now
• all commands are working with callbacks now, because there is no more synchron tss.js process
• commands in the tss.js execution queue will be reordered, so that a showErrors will always work with the newest data
• commands in the tss.js execution queue will be merged, so even if there are 3 showErrors in the queue, it will only be executed once
• debouncing still enabled: showErrors will wait 0.8 seconds after the last keystroke
• showErrors will only be executed if anything has changed on the open files.

This results in a much better responsiveness. Previously i had some delays (gui hang ups) on completion trigger etc, but now it works much smoother.

More changes:

• refactoring, renaming, documentation of some files
• Debug print statements in code

All these changes are working well with Linux and sublime 3. When i'm done i can test it with sublime 2 (if they have a free version). Can you test it in windows?

I'm open to any suggestions on my changes.

regargs, Dän

[Railk]

Hello @Phaiax,

I've begin to look at your change yesterday and i'll try to test a look on windows and mac osx with your change this week-end.

You're change are really interesting :) and address problems i was trying to solve (i was considering going full python by translating typescript language service in python, but that's quite a task and a bit of maintenance problem after, to solve the laguiness)

if everything work well i'll merge asap. (i'll try to do that this week end)

thx for your contribution :)

[Phaiax]

In theory: Should the dev branch still work with sublime 2 or are all these ST3 statements just old crap? Because plugin loading will fail directly in T3S.py on the first import statement for ST2 in line 16:

from lib.Commands import * with ImportError: No module named lib.Commands

[csnover]

From an end-user perspective, this PR is great and seems to solve a severe performance problem in T3S that was forcing me to not use it. However, it needs at least the following two changes, otherwise it is very broken:

diff --git a/lib/Listener.py b/lib/Listener.py
index 51bc390..0fd15e6 100755
--- a/lib/Listener.py
+++ b/lib/Listener.py
@@ -137,7 +137,7 @@ class TypescriptEventListener(sublime_plugin.EventListener):
                COMPLETION.trigger(view, TSS)

                if not SETTINGS.get('error_on_save_only'):
-                       TSS.errors(view.file_name)
+                       TSS.errors(view.file_name())
                        #debounce(TSS.errors, self.error_delay, 'errors' + str(id(TSS)), view.file_name())


diff --git a/lib/Utils.py b/lib/Utils.py
index 5285e50..ce39e33 100755
--- a/lib/Utils.py
+++ b/lib/Utils.py
@@ -156,7 +156,7 @@ def read_file(filename):

 def read_and_decode_json_file(filename):
        """ returns None or json-decoded file contents as object,list,... """
-       f = read_file(f)
+       f = read_file(filename)
        return json.loads(f) if f is not None else None

 # FILE EXISTS

[Railk]

i'll do the testing/merging next week as i'll be on holiday, sorry for the long delay.

[Phaiax]

There is another thing I want to implement, the auto completer has to check wheather it's the same dot it belongs to.

Type:
> this.          -> autocomplete triggers
>       foo.    -> autocomplete is ready and now displays the suggetions for this.

Thats sometimes confusing, but I will adress this issue during the next days. And I will include csnovers changes.

Ah, and the errorview to source linkage is misbehaving. If something is underlined red (error), but you resolve the issue in another file, the red underlining will remain until you make the next change to that first file.

Btw, I think its a good idea to indicate that there is a pending calculation of the current errors. Maybe I will use the first line of the error view to archive this.

[csnover]

Here is another error that came up at some point which I assume is related to this PR:

File navigation : string indices must be integers
Traceback (most recent call last):
  File "T3S/lib/Commands.py", line 201, in async_react
    start_line = member['min']['line']
TypeError: string indices must be integers

This happened after doing an F5 project refresh.

Subsequent to that, when closing the project:

Traceback (most recent call last):
  File "T3S/lib/system/AsyncCommand.py", line 141, in <lambda>
    sublime.set_timeout(lambda:self.result_callback(tss_answer, **self.callback_kwargs),000)
  File "T3S/lib/Tss.py", line 223, in kill_and_remove
    PROCESSES.kill_and_remove(root)
  File "T3S/lib/system/Processes.py", line 84, in kill_and_remove
    self.get(root, SLOW).kill_tssjs_queue_and_adapter()
NameError: global name 'SLOW' is not defined

[Phaiax]

I will hopefully fix that this weekend

[Phaiax]

In the last commits I introduced some errors, fixed hopefully all of them.

Except for this error I could fix everything csnover mentioned. Thank you a lot

File navigation : string indices must be integers Traceback (most recent call last): File "T3S/lib/Commands.py", line 201, in async_react start_line = member['min']['line'] TypeError: string indices must be integers

Changes:

• the squiggly underline at errors is now not only refreshed in the current document but in all open ts files
• the first line of the error view indicates
• ..

----- cut I changed a little bit of the code (to always bring the outline/error views to the top if the hotkeys are pressed), but now (i don't know where, and maybe its an issue in sublimes plugin_host) on certain conditions the process eats up memory very fast (Gigabytes per second). There is a debugger for sublime addons, but i don't get the point :(

[Phaiax]

I fixed the problem by refactoring a lot :) I took a look at when some of the functions were called and there were multiple calls to things which needed only one call. So i decided to change the structure a bit (all regarding the outline and error view and the error highlighter). I think it's fine and I hope it's fine for you too.

Just take a look, its finished so far (all features included, maybe some bugs left)

[csnover]

Thanks, I’ll reapply in the next while and let you know how it works here.

[csnover]

So far so good! I’ll continue banging on this the next couple days and let you know if I run into anything. Thanks for the hard work!

[Phaiax]

I just had an internal tss.js error: Cannot read property 'lineMap' of null The reason was, that I had remapped the project directories in my Typescript Project in the .sublime-project file.

  {
        "name":  "MY SRC",
        "path": "src",
        "file_exclude_patterns": ["*.js"]
    },
    {
        "name":  "ALL",
        "follow_symlinks": true,
        "path": ".",
        "folder_exclude_patterns": ["node_modules", "phaser", "build"],
    },
    {
        "name": "PHASER SRC",
        "path": "phaser/src",
    }

Are there many people doing this?

[Phaiax]

The problem was easier to solve than I’ve thought first.

[Phaiax]

I caught myself only using my mouse to click on the errors in the error view. The error key command has changed from ctrl+shift+e to shift+alt+e, because the first one is hart to press, you have to turn your hand (I know everybody can override this, but in my opinion this is a bad default)

With the combinations: shift+alt+e,shift+alt+h it jumps to the first error in the error list. By pressing j, k, l you get to the 2nd, 3rd, 4th error respectively.

Todo: Documentation

[csnover]

I don’t know if you plan on addressing it, but the one issue that still exists for me (that existed and was worse before this patch) is where the imported dependencies are not correctly loaded, resulting in files that display false errors because tss thinks that an imported module is a “non-module type”. Do you know what is going on there?

Other than that, I’ve continued to have great success with this branch. Much better than what exists in the repo. This should land.

[Phaiax]

I only experienced those import errors in files which are not imported by any other file. Those files will still be added and updated to the typescriptservice and I think it causes undefined behaviour.

Is there any other case where those undefined imports can occour?

There is also this other issue which has been in T3S before: you can define multiple root files but only the first one will ever be loaded or used.

I think both problems are relatively easy to resolve, but may cause some bugs to appear which are harder to solve. Ill take a look.

On July 24, 2014 5:28:37 PM CEST, Colin Snover [email protected] wrote:

I don’t know if you plan on addressing it, but the one issue that still exists for me (that existed and was worse before this patch) is where the imported dependencies are not correctly loaded, resulting in files that display false errors because tss thinks that an imported module is a “non-module type”. Do you know what is going on there?

---

Reply to this email directly or view it on GitHub: https://github.com/Railk/T3S/pull/63#issuecomment-50033872

Diese Nachricht wurde von meinem Android-Mobiltelefon mit K-9 Mail gesendet.

[csnover]

One way that seems to be possible to consistently reproduce the issue is:

1. Ensure hot_exit is enabled in Sublime (it is by default)
2. Open a TypeScript file that has an import statement
3. Note that import loads OK and there are no errors
4. Quit & reopen Sublime
5. Note that import has not loaded and there are errors related to import
6. Close & reopen tab
7. Note that import loads OK and there are no errors

With regards to the root files…I honestly don’t understand why they are a thing that exists. When I open a TypeScript file, I want code intel for that file. The files that need to load are any open TypeScript file, plus the opened files’ explicitly defined dependencies (/// comments, import statements). I don’t understand what circumstances exist that one should have to specify a “root” file (badly written code without explicit dependencies?).

[Phaiax]

The root files are something that is related to typescript itself. If you type a compile command, you specify 'the root file'.

tsc "/home/foo/baar/main.ts" --module amd --target ES5 --outDir ...foobar...../build/js

Because typescript allows for circular imports, there is no logical way to determine the correct root file. Your only could get a clue to the directory, if you try to find the directory where the imports are relative to. But how far should you search? And what if you first open a library file located somewhere else.

So currently, if you open a typescript file and you did not specified or specified a non existent or wrong root file, it can't do anything. (The current problem is that he does something).

So for your example of reproducing: Do you have a root file specified and does that root file import the file you open? Do you have a .sublime-project file?

I will create some testprojects in the repository with all possible valid and invalid combinations just to be able to test.

[csnover]

Because typescript allows for circular imports, there is no logical way to determine the correct root file.

I believe the “root” file would be the file that is open in the currently focused tab.

Circular dependencies between types are fine in TypeScript, even between those that are imported. AFAIK the only time there is a circular dependency problem is when you load external modules at runtime, where the order in which they are loaded determines which module is treated as the circular dependency (a -> b -> a or b -> a -> b). This is not a problem in the compiler, but a limitation of the way that JavaScript works. It should not impact the static analysis of TypeScript.

Your only could get a clue to the directory, if you try to find the directory where the imports are relative to. But how far should you search? And what if you first open a library file located somewhere else.

The way the TypeScript compiler resolves dependencies is very straightforward, and if it doesn’t already, T3S should take the same approach to ensure correct interoperability.

For absolute module IDs, the compiler looks for <module id> + '.ts', then <module id> + '.d.ts', up the directory chain starting from the directory of the dependent file until it reaches the root of the filesystem.

For relative module IDs, you just resolve the relative ID relative to the dirname of the dependent file, <dirname> + <module id> + '.ts', then <dirname> + <module id> + '.d.ts'.

An approach where the open files in the editor are the ones that are used as dependencies for other open files in the editor leads to an incredibly brittle and badly functioning design, one that does not match how the compiler works and one that leads to false positives & false negatives in the editor.

So currently, if you open a typescript file and you did not specified or specified a non existent or wrong root file, it can't do anything. (The current problem is that he does something).

I don’t know what this means; I have to do this all the time (using a bogus root file) and it pretty much works. I am a library developer; there is no “root file” for a library.

So for your example of reproducing: Do you have a root file specified and does that root file import the file you open? Do you have a .sublime-project file?

I have a .sublimets with a bogus root file.

[Phaiax]

Ok, I've thought about it and tested what the typescript-services are capable of:

At first It's no general problem to use the first open .ts file to init the typescript-services. The only drawback is the following situation: You have a (big) project and most of the files are closed.

lets say the imports are as following: main.ts -> lib.ts -> utils.ts

1. You open lib.ts.
2. The typescript-services init with this file. (utils.ts will be added to the internal model of the applcation; main.ts will not be recognized by the typescript-services)
3. You change a function name in lib.ts. No error will be displayed for any now invalid function call in main.ts

The error would be recognized if the typescript-services were always initialized with the file from which all imports are starting.

And I was able to reconstruct your error from above:

Files: main.ts (root) >imports> lib.ts anotherfile.ts >imports> utils.ts Open editor with the file anotherfile.ts first. The import of utils.ts will fail. That's a 'bug' in the typescript-services. In theory it would be no problem to just search for that imported file and add it. But it doesn't do so, so we have to deal with it.

It only automatically adds files which have an import-line-of-sight to the "root"-file the typescript-services were inited with.

There are two ways to resolve this:

• First like you did with reopening: Just open utils.ts in another tab. T3S currently adds every .ts file to the typescript-services, so this will cause the error to disappear.
• Creating an import-of-sight to the rootfile: maybe by importing anotherfile.ts in lib.ts. That error will still remain, even if you trigger error-compilation again by saving or adding a space in a random .ts file. Only if you hit F5 afterwards (which triggers the 'reload' command of the typescript-services), the typescript-services will probably recreate their import tree and the error will disappear.

So i draw the following conclusions. Correct me if you think i'm wrong.

• Rootfiles are necessary to import every file at init time to be able to make error detection in all files (first example)
• There is no need to force people to use root files. If they have a stand-alone (or two or three..) .ts files they probably only want some error correction for that file.
• Even for those people it's very simple to just create a .sublimets file with the wizard.

so

• I would still force people to create a typescript project or a .sublimets file.
• I would update the documentation about rootfiles to something like this:
  • Rootfiles: You must set a rootfile. Use that file as rootfile which directly or indirectly imports any other typescript file in your project. You can specify multiple rootfiles if you have multiple roots in your import tree.
• I would display additional hints in the errorview if import error statements are detected:
  • // Make sure you have a straight chain of imports from any of your rootfile to {the file with the error}. Then press F5 (aka Typescript: Reload command in command palette)
  • // If you don't want that straight chain of imports, just add another rootfile to your sublime-project. You maybe need to convert your .sublimets file to a <>.sublime-project file. Refer to the T3S Readme.

I think that would be ok for everyone, even you as a library developer.

Btw: you can take a look at the watched files aka the files in the import tree of typescript-services. Type this into the python console: import T3S T3S.lib.system.Files.FILES.update_indexed_files(sublime.active_window().active_view().file_name()) print("\n".join(T3S.lib.system.Liste.LISTE.liste.keys()))

I also fixed a bug which caused an error in when T3S tried to refer to the settings for a non-indexed .ts file. I will push soon.

[Phaiax]

@Railk I would be pleased if you could comment my last comment. And maybe you can explain what rootfiles are for in your opinion, or explain me the concept in a few words.

[Railk]

Sorry life happened, my free time went away and i've been busy since. I'll comment your last comment as soon as possible, i prefer not to give time schedule right now but it's high on my todo list to check everything, i already began the merge and test on my local machine but could not finish yet...

[csnover]

@Phaiax Thanks for your thoughtful response. I’m planning to read through it in greater detail and respond to any points when I have a free moment.

[pocesar]

@Phaiax can you merge the #64 PR and release the package on Package Control under a new name?

[Phaiax]

I can do this, but i'm in the usa currently without my laptop, so you'll have to wait until november.

Diese Nachricht wurde von meinem Android-Mobiltelefon mit K-9 Mail gesendet.

[pocesar]

@Phaiax thanks, that would be great :+1:

[alexgorbatchev]

ping

[Phaiax]

I'm trying it in the next few days, but no promise :D

[KATT]

pong

[Phaiax]

Jep i know.. I'm participating in an Hackathon on Friday so its done this weekend probably

Diese Nachricht wurde von meinem Android-Mobiltelefon mit K-9 Mail gesendet.

[Phaiax]

Hey

I've created a new plugin, which is based on T3S and includes these patches.

https://github.com/Phaiax/ArcticTypescript

You can install ArcticTypescript using PackageControl. Please remove T3S first, both plugins together are incompatible.

• untested on windows and mac, but i will fix bugs as soon as you create Issues for ArcticTypescript.
• major updates which will break configuration structure may happen in future

Phaiax

[druppy]

@Phaiax thanks, this seems like a god plan, I like your #63 a lot so this seems like more of that ! ArcticTypescript ... here I come :-)