Normalize dependency graph paths - Fix broken dependencies on Windows

[pzavolinsky]

instance.dependencyGraph gets created using a raw containingFile value:

var servicesHost = {
    // ...
    resolveModuleNames: (moduleNames: string[], containingFile: string) => {
        // ...
        instance.dependencyGraph[containingFile] = ...
        // ...
    }
    // ...
}

But is later accessed with a normalized path:

function loader(contents) {
    // ...
    var filePath = path.normalize(this.resourcePath);
    // ...
    let additionalDependencies = instance.dependencyGraph[filePath];
    // ...
}

This breaks every scenario where path.normalize is not identity.

For example, in Windows, you get:

containingFile: 'C:/project/dir/file.ts'

path.normalize(containingFile): 'C:\\project\\dir\\file.ts'

[johnnyreilly]

Hi @pzavolinsky,

could you refork and resubmit this PR please? I've just got CI cleaned so the tests pass but i can't requeue a build for your PR manually...

[pzavolinsky]

Hi @johnnyreilly , I've just rebased my fork against ts-loader/master. You should be able to merge without conflicts now.

[johnnyreilly]

Hi @pzavolinsky,

Thanks for submitting that - happily the tests are passing with your change in place. 😄 The change looks good.

Could you tell me how you discovered this issue? I'm guessing there was some kind of a problem that you encountered.

I ask as I'm wondering if it may be worth adding a test to ensure there are no future regressions.

[pzavolinsky]

Could you tell me how you discovered this issue? I'm guessing there was some kind of a problem that you encountered.

Sure, here it goes:

We have lots of files called types.ts that define just reusable types. ts-loader was not picking up changes in those files. After a bit of debugging I found the path normalization issue and that dependencyGraph lookup always failing.

There must be something special going on for types.ts files, because other files work just fine, so you might want to look into that too.

Hope this helps!

[johnnyreilly]

Thanks for the explanation @pzavolinsky. Very helpful. I'm not too sure if there's a good way to cover this with a test but I want to have a think first. Assuming I don't come with anything I'll plan to merge this.

Thanks again!

[pzavolinsky]

Great, let me know if you need anything else!

[johnnyreilly]

Awesome - thanks!