Fixed issue with incorrect output path for declaration files

[JonWallsten]

In regards of: #190

I'll try to explain the fix:
Below is the code where we prepare the relative path for the d.ts files so we can add them to Webpack's compilation assets.

const assetPath = path.relative(
        compilation.compiler.context,
        declarationFile.name
      );
compilation.assets[assetPath] = {
        source: () => declarationFile.text,
        size: () => declarationFile.text.length
      };

Example
Project root: /home/user/jon/projects/a/
[webpack.main]: index.ts
[webpack.output.path]: /home/user/jon/projects/a/dist/
[compilerOptions.declarationDir]: ./dist/typings/

[Scope variables]
[compilation.compiler.context]: /home/user/jon/projects/a/
[compilation.compiler.outputPath]: /home/user/jon/projects/a/dist/

[Other]
All relative assets in [compilation.assets] will have [compilation.compiler.outputPath] as base it seems.

Results
What will happen here is that path.relative will return a relative path with compiler.context (project root) as a base. So when TypeScript compiler outputs /home/user/jon/projects/a/dist/index.d.ts (declarationDir is relative to the project root), path.relative will strip the project root and we will end up with dist/index.d.ts. It looks correct, right? Well, if we would store the compiled files relative to the project root it would. The issue is that compilation.assets seems to equal webpack.output.path.
So we'll end up with this:

/home/user/jon/projects/a/dist/index.js
/home/user/jon/projects/a/dist/dist/index.d.ts

By changing compilation.compiler.context -> compilation.compiler.outputPath we will get a path relative to the output folder instead.

So if we have the same output from the TypeScript compiler: /home/user/jon/projects/a/dist/index.d.ts and we strip away /home/user/jon/projects/a/dist/ instead, we end up with index.d.ts. The result would be:

/home/user/jon/projects/a/dist/index.js
/home/user/jon/projects/a/dist/index.d.ts

Note that this might be a breaking change since people probably has workarounds or have adapted to this way. Myself included.

[johnnyreilly]

I really appreciate the detailed PR - that's very helpful indeed!

Note that this might be a breaking change since people probably has workarounds or have adapted to this way. Myself included.

Could you provide some details around what workarounds people might be making use of?

[JonWallsten]

I just mean that people have adapted to use a path relative to the output folder instead of the root.
My project for example:
The packages where I still use ATL have './dist/typings'
The packages where I use TS-loader have just 'typings'
Now they will end up in the same output folder. The problem with the ts-loader however is that when I run tsc manually, for example when I compile the files for testing, the output folder will be /typings/ instead of /dist/typings/. And as you know you can't unfortunately have different tsconfig files in the same project. That's how I found the issue I replied to.

I thought I had during the night was that maybe we can combine the old solution and the new one. If you start your declarationDir with dot-slash, './output', we could use the project root as base. And if you just use 'output' for example it could be relative to the webpack output folder. Still a bit confusing though since tsc wouldn't output the files in the same folder.

[johnnyreilly]

Cool. Essentially I'm trying to evaluate how "breaking" this change would be. If it's not really that break-y then I'd consider treating this as a fix version-wise. Alternatively we'd have to do a major version bump which I'm less keen on, but if it's appropriate then that's what it should be. Do you have a view?

[JonWallsten]

Yeah, a major seems unnecessary for this small fix. But still I might require action from the user. I'm not sure what the conventional thing to do is.
I'm not in a hurry with this fix, so take your time and let me know. Whatever works for me!

[johnnyreilly]

Thanks chap. I'm not going to be online much for the next couple of weeks; I'll probably look to ship this with #817 when the time comes.

[JonWallsten]

Sound good! Just ping me when the time comes!

[gluons]

Thank you very much for this fix. 😍

I've tried this PR. It works properly. (Both with outDir and declarationDir options in tsconfig.json) 🎉

But it works only if I set outDir (or declarationDir) in tsconfig.json (e.g. dist).

Image copied from #190

---

If I forget to set outDir (or declarationDir), all declaration files will be placed at the same place as source files.

Image copied from #190

Maybe we need note for reminding users to set outDir or declarationDir.

It should be great if outDir/declarationDir isn't required. But this behavior is OK for me.

[JonWallsten]

@gluons I would have to say that it's intended (but maybe unwanted in some cases?). It's TypeScripts default behavior to do this. If you don't add outDir or declarationDir it might be intended, and you could get annoyed by a warning, since it's in the context of TypeScript isn't wrong. What does @johnnyreilly think?

[johnnyreilly]

It's TypeScripts default behavior to do this.

Then that's what ts-loader should do 😁

As a general rule of thumb, ts-loader aims to be a drop in replacement for tsc behaviour wise; only deviating if there's specific cause. I don't see that in this case so I think it's just fine.

BTW I'm hoping to get this merged and shipped either next week or the week after.

[gluons]

OK. I get it.
It's actually fixed now. 🎉

[johnnyreilly]

Hey @JonWallsten,

I'm just heading back from holidays. I've been thinking; although this is a minor change, you're right that it's technically a breaking change. As much as I don't want to bump the version number; ts-loader seeks to be a good semver citizen. So it's time to bite the bullet.

Please could you:

1. Update the version in the package.json to 5.0.0
2. Add a corresponding entry to the CHANGELOG.md

Then I'll close my eyes, weep a bit and push out the major version. Cheers!

[JonWallsten]

@johnnyreilly Sure. I'll fix it before nightfall! And don't worry, it seems like it's fashion to bump majors all the time now anyways ;) I mean, look at Chrome 68!

[johnnyreilly]

Thanks for your work @JonWallsten!

Released with https://github.com/TypeStrong/ts-loader/releases/tag/v5.0.0

[JonWallsten]

Perfect! No worries! Happy to help for my own cause! ;)

[Jogai]

I mean, look at Chrome 68!

You mean 69?