clausreinke / typescript-tools

(repo no longer active) Tools related to the TypeScript language
Apache License 2.0
266 stars 29 forks source link

Reference file error #20

Closed Railk closed 10 years ago

Railk commented 10 years ago

Hello,

I have a problem with reference file (i'm sure i've seen a discution about this problem, but cannot find it in the closed issues. Sorry if there's already an answer to that)

  1. I load a file with a reference file that cannot be resolved, showErrorsindicate the error.
  2. I then update this reference file name to be correct and use showErrors and i can still see the error.
  3. i reload the project and use ShowErrors and can still see the error.

Should i do something more to do not have the error showing after the reload ?

Bash@ /d/X_DEVELOPPEMENT_X/jscipt_developpement/TYPESCRIPT/TSS/reference $ tss main.ts
"loaded d:/X_DEVELOPPEMENT_X/jscipt_developpement/TYPESCRIPT/TSS/reference/main.ts, TSS listening.."

showErrors
[{"file":"d:/X_DEVELOPPEMENT_X/jscipt_developpement/TYPESCRIPT/TSS/reference/main.ts","start":{"line":1,"character":1},"end":{"line":1,"character":42},"text":"error TS5007: Cannot resolve referenced file: 'underscore-.d.ts'.","phase":"Resolution","category":"Error"}]

> update 1 main.ts
/// <reference path="underscore-1.4.d.ts" />
"updated d:/X_DEVELOPPEMENT_X/jscipt_developpement/TYPESCRIPT/TSS/reference/main.ts, (0/0) errors"

showErrors
[{"file":"d:/X_DEVELOPPEMENT_X/jscipt_developpement/TYPESCRIPT/TSS/reference/main.ts","start":{"line":1,"character":1},"end":{"line":1,"character":42},"text":"error TS5007: Cannot resolve referenced file: 'underscore-.d.ts'.","phase":"Resolution","category":"Error"}]

> dump ouput.txt main.ts
"dumped d:/X_DEVELOPPEMENT_X/jscipt_developpement/TYPESCRIPT/TSS/reference/main.ts to ouput.txt"

reload
"reloaded d:/X_DEVELOPPEMENT_X/jscipt_developpement/TYPESCRIPT/TSS/reference/main.ts, TSS listening.."

showErrors
[{"file":"d:/X_DEVELOPPEMENT_X/jscipt_developpement/TYPESCRIPT/TSS/reference/main.ts","start":{"line":1,"character":1},"end":{"line":1,"character":42},"text":"error TS5007: Cannot resolve referenced file: 'underscore-.d.ts'.","phase":"Resolution","category":"Error"}]

main.ts

/// <reference path="underscore-.d.ts" />

updated to:

/// <reference path="underscore-1.4.d.ts" />

underscore-1.4.d.ts

declare var _:string;
clausreinke commented 10 years ago

Thanks for the clear report. I don't think this is a bug, though it is a bit awkward.

update will not trigger dependency resolution, so the old resolution error messages remain, too.

reload does dependency resolution, but reloads source code from the file system, so it misses any in-memory updates you've done. So you'll get a fresh copy of the original errors (same as before the update).

It did surprise me when I first encountered this combination, but I think it is best handled in the client. In the Vim plugin, I block reload if there are still modified buffers for in-project files.

draivin commented 10 years ago

I had the same issue as well, my "solution" was to detect if the user did any change that would require a reload, and then automatic reload the project on a file save, this way I don't have to expose the reload command directly to the user, and the only drawbacks is that the import errors are only updated when the file is saved (not a real drawback compared to manually reloading tough).

Probably the discussion you have seen was when I hijacked an issue of yours: https://github.com/clausreinke/typescript-tools/issues/17#issuecomment-24474247

Railk commented 10 years ago

@clausreinke Yes i handle that in the client side :), but like it felt a bit strange i prefered to ask in case there was another solution. Thanks for your quick answer.

@Draivin Ah yes i know i saw that somewhere :) On my local dev version i also reload when it's necessary now, and do not oblige the user to reload by himself.

clausreinke commented 10 years ago

The situation is certainly not ideal.

My main reason for implementing update was to ensure that completions was working on the most current source. reload is the safest way to get everything in synch, but it is rather drastic and takes too long on non-trivial projects (several seconds), so would be good to have a faster way to get some error messages. update fits somewhere in this role, but doesn't fill it completely (as you've noticed). I was thinking of implicitly triggering reloads in the background, asynchronously, but now there is a conflict between in-memory updates and on-file source...

It doesn't help that TS modularity is partially include-based, so instead of clean in-file dependencies, a file can depend on being loaded after another file that it has no explicit dependencies on. Which means that partial reloads would be difficult, at best.

It seems that quite a few IDE issues on the TS tracker are due to this same conflict and bad interactions between fast and background feedback.

draivin commented 10 years ago

Couldn't you provide a flag for reload that would use the latest updated version of the root file instead of the one on disk, and when chasing dependencies would do the same, when all dependencies have been loaded, you would remove the files that are not included anymore from memory.

A pseudo implementation:

this.updatedFiles = {};

function update(file, content) {
 this.updatedFiles[file] = content;
 ...
}

function reload(keepUpdates) {
  this.keepUpdates = keepUpdates;
  ...
}

function getScriptSnapshot(file) {
  if(this.keepUpdates && this.updatedFiles[file]) {
    return TypeScript.ScriptSnapshot.fromString(this.updatedFiles[file]);
  }

  ...
}

function setup(file) {
  ...
  if(this.keepUpdates) {
    remainingFiles = {};
    resolvedFiles.forEach(function(code) {
      if(this.updatedFiles[code.path]) remainingFiles[code.path] = this.updatedFiles[code.path];
    });
    this.updatedFiles = remainingFiles;
  } else {
    this.updatedFiles = {};
  }
}

This would allow to reload without saving while keeping the updates.

clausreinke commented 10 years ago

That would always give updates priority over the file system - what if the file system version is newer than the update? What if the updated file does not exist on disk? etc. Also, when taking the time hit of a full reload, isn't that a good time to save all the in-memory updates to disk anyway?

But, yes, there are options - in particular, I would like to skip reprocessing parts of the dependency (non-)tree that haven't changed (whether by newer file or update), to get something faster than a full reload (if possible, I'd also like that to be async, so that the client could continue editing while tss is chasing dependencies; although that part seems to be problematic in TS IDEs). There is support for partial reprocessing somewhere in the language service, I just need to figure out how to combine it with partial reloading. So far, it is on my think-about-it list, not yet on my todo list;-)

draivin commented 10 years ago

I believe it is the TSS consumer task to worry about the file state, and unless the end user is editing his typescript files with different editors simultaneously, the update state would always be the correct one.

About the saving on full reload, I don't really feel confortable saving files on the user behalf, but I still have to update the errors on my side. I'm fine with waiting for the user to save the file himself so I can update the errors, but it would be great if I could update them without having to wait.

Other than that, any way to make reloads take less time would be nice, altought I am a bit afraid of async because the state can be unpredictable, how do you know when the reload finished and the loaded files have changed? It is nice having the guarantee that TSS's state will only change when I send a command.