TypeScript 3 peerDependencies warning

[nickserv]

If this project officially supports TypeScript 3, its peer dependencies should be updated to allow it.

[johnnyreilly]

Not a bad idea. But I think actually removing them from peerdeps would be better: #109

[nickserv]

I wasn't aware ts-loader didn't have a peer dependency on typescript, do you know why? Peer dependencies are meant to signify dependencies used with plugins and tools that can have different versions, removing it from peer dependencies is not a good idea unless this plugin can somehow use all of typescript's features without typescript installed, like with a remote web service.

[johnnyreilly]

I'm not completely sure - sorry. That said, given peerDependencies only trigger a warning in an (already noisy) install log I don't think they provide a ton of value. Particularly given the current situation which has prompted this discussion: i.e. there's a new version of TypeScript and because of the major version increment, we now need to ship a new version of fork-ts-checker-webpack-plugin just to silence the noise. Yeah, it's situations like this that make me less keen on peerDependencies!

[nickserv]

I agree that the noise isn't ideal, but as I mentioned in #102 (comment) I think the safest option is to continue using peerDependencies, and it makes more sense to me semantically. If I understand correctly, this package only works when TypeScript is manually installed by the user, and peerDependencies remind users to install shared dependencies manually. If you want to automatically install peerDependencies in npm 3+, there's a handy install-peerdeps package. Note that peerDependencies on TypeScript are typical in type packages and tools like TSLint.

If you don't want to have to update the peerDependencies to support new major versions, it's best to use >= ranges which only specify a minimum version. This would also solve our immediate issue with TypeScript 3, assuming there are no major issues with it, which I haven't noticed yet using CRA-TS with this package.

"peerDependencies": {
  "tslint": ">= 4.0.0",
  "typescript": ">= 2.1.0",
  "webpack": ">= 2.3.0"
}

[johnnyreilly]

If you want to submit a PR with >= ranges then we'd take a look. I think I'm still inclined to remove peerDependencies entirely one day. But for now I'd just rather remove noise from users.

[piotr-oles]

I would not agree with you. I think it depends on user preferences - for example, I always check peer dependencies when I install a new package. I remember when webpack bumped version to 3.0.0 we had to do some changes in this plugin to make it compatible so it's not only dumb warning :)

[johnnyreilly]

Okay cool - so shall we merge this PR? 😄

[nickserv]

@piotr-oles peerDependencies says what OTHER dependencies should be, and npm no longer installs them automatically. This means that updating a restriction to support a new version of TS still requires a manual upgrade to the new version of TS separately, so you would still be aware of breaking changes in other packages. That being said, peerDependencies should still reflect what the package that has them (this one) is known to support, though I've seen some packages use less restrictive version ranges and warn users in the readme when some versions haven't been officially tested.