Problems in dependency resolution

[s-panferov]

Hello @andreypopp. Thanks for the great library! I have been using it a little and I have found several big problems. The main problem right now is that is doesn't collect all the files that TypeScript needs. Consider this code from your library:

https://github.com/andreypopp/typescript-loader/blob/master/lib/TypeScriptWebpackHost.js#L177-L193

TypeScriptWebpackHost.prototype._addDependencies = function(resolver, filename) {
  var dependencies = this._findImportDeclarations(filename).map(function(dep) {
    return this._resolve(resolver, filename, dep).then(function(filename) {
      var alreadyExists = this._files[filename];
      var added = this._readFileAndAdd(filename);
      // This is d.ts which doesn't go through typescript-loader separately so
      // we should take care of it by analyzing its dependencies here.
      if (/\.d.ts$/.exec(filename) && !alreadyExists) {
        added = added.then(function() {
          return this._addDependencies(resolver, filename);
        }.bind(this));
      }
      return added;
    }.bind(this));
  }.bind(this));
  return Promise.all(dependencies);
}

In this function you traverse only one level of the regular (non .d.ts) dependencies. But to compile the file TS needs the whole dependency tree to be loaded into the service.

We don't see any errors because here you collect only current file diagnostics, but the actual errors are in the file deps.

Several days ago I have forked your library to awesome-typescript-loader and completely rewritten all the things there.

I wrote this issue because:

1. I want to warn you and your users about the problem.
2. If you want I can explain all the changes I made there so you could port them to your library if you want.

I added the info about your authorship into the README, but your project doesn't contain the LICENSE file so just say if you want me to add some additional copyright notice.

[andreypopp]

Hm... do you a have repro case? I don't traverse non .d.ts because Webpack will call typescript-loader on them so I don't need to recurse into the tree myself.

[s-panferov]

@andreypopp Consider such case:

fake/auth.ts:

// spelling mistake here
export function mocAuth() {

}

fake/index.ts:

export import auth = require("./fake/auth");

app.ts:

import fake = require("./fake/index");

// You will not see the error here
fake.auth.mockAuth()

Here we have app.ts -> fake/index.ts -> fake/auth.ts chain. You have only app.ts -> fake/index.ts while compiling app.ts, so TS can't see the error. Instead, it puts fake/auth.ts not found into fake/index.ts errors. But you don't show it because here you don't ask about deps.

[s-panferov]

Note that emit continues, because there are no errors in app.ts itself. And when it compiles fake/index.ts it has the full chain already, so there it no errors really in fake/index.ts. So we don't see any errors in the output.

But the major problem is that it don't have the whole chain to type-check app.ts completely.

[jbrantly]

@s-panferov Hopefully this isn't in too bad of taste, but I solved this problem by just outsourcing all of this to the typescript compiler. That way it works identical to tsc and the code is a lot simpler/shorter. This has been discussed quite a bit before, like in #2 and in webpack/webpack#61

[s-panferov]

Hello @jbrantly. I saw your https://github.com/jbrantly/ts-loader and it's amazing! But I have tested it for the above scenario and it doesn't show errors too because the code is the same:

https://github.com/jbrantly/ts-loader/blob/master/index.ts#L173-L174

    var diagnostics = langService.getCompilerOptionsDiagnostics()
        .concat(langService.getSyntacticDiagnostics(filePath))
        .concat(langService.getSemanticDiagnostics(filePath))

You ask TS only for the current file diagnostics.

Another big problem is the --watch mode. Both ts-loader and typescript-loader don't support it as I can see. By making my own dependency resolution system I have achieved the ability to recompile only changed files and the files that are dependent (even in chain). I have found a lot of troubles on the way, and I continue the work, but I think that it is already works for most cases.

Maybe it can be done simpler by using TypeScript compiler?