Enables custom resolver only if they're used in incremental mode

[arcanis]

This diff only adds the custom resolveModuleNames wrapper if the custom resolution is really used. It theoretically shouldn't be needed (we should be able to implement the default resolution), but there's a problem in TS with the watch system.

Since it's only about the native TS watch system, I've only made the change in CompilerHost (not in IncrementalWatcher).

Tests are a bit more funky since it requires a plugin. Thoughts?

[arcanis]

Note that it won't fix it for people w/ custom resolvers, but at the moment that's only PnP users and that probably can be considered a caveat.

[arcanis]

Opened an issue on the TS repo to discuss more long-term fixes: microsoft/TypeScript#31056

[johnnyreilly]

Hey @arcanis,

I've read this through a bunch of times but I think I'm not quite following it. Or I may be but I'm not certain I understand correctly. If it's cool with you I'll make a bunch of statements and you can agree / set me straight as necessary:

1. This PR fixes tsconfig paths resolution not working in v.1.1.0 #258
2. PnP will work without the incremental API
3. PnP will not work with the incremental API. This is a problem that only be resolved by changes in the TypeScript watch API.

Regarding tests, I think integration tests are the way to go here.

Essentially, if we have a test something like this: https://github.com/Realytics/fork-ts-checker-webpack-plugin/blob/dfd8933f23da7ca72aa3a864e5811725146323d4/test/integration/index.spec.js#L122

Which detects an error in an alias path module we should be good to go. And obviously we'd only run this against the non-incremental API

[arcanis]

No it's less serious than this:

• This PR fixes tsconfig paths resolution not working in v.1.1.0 #258
• Using paths with PnP will work without the incremental API.
• Using paths with PnP will not work with the incremental API. This is a problem that only be resolved by changes in the TypeScript watch API.

Basically the options are still not forwarded properly when having custom resolvers w/ the incremental API, but the consequences now only affect people that use custom resolvers.

[arcanis]

Should be good now. I've still put the test in the two modes but marked it as skipped in incremental mode, as it's something that should work in the future.

[johnnyreilly]

Great! This should go out with https://github.com/Realytics/fork-ts-checker-webpack-plugin/releases/tag/v1.1.1

BTW @piotr-oles is switching the repo over to use semantic releases and so I think this may be the last release to go out this way!

#251 (comment)

[piotr-oles]

🎉 This PR is included in version 1.2.0-beta.3 🎉

The release is available on:

• npm package (@beta dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[johnnyreilly]

The thing I'm going to miss now we're in semantic release land is the ability to raise mini PRs and ship releases from my phone. I'm always going to need a keyboard and terminal from now I guess...

Still.... Emoji in commits 😎

[piotr-oles]

Why do you need a terminal? :) You can create a pull request and type proper commit message and semantic-release will do the work for you. Am I missing a point?

[johnnyreilly]

Maybe you're right... I guess how strict is "proper commit message"? Looking at a sample commit:

fix: 🐛 semantic-release update CHANGELOG.md on the git repo

Do I need the " 🐛"? Because I'm going to struggle finding that on my phone keyboard 😁

Maybe it's something I'll learn over time; but doesn't semantic release recommend using their commit script: https://github.com/Realytics/fork-ts-checker-webpack-plugin/blob/71bde90ead3661decede2b947111fdd741571662/package.json#L49

That made me think "wow this is clearly easy to get wrong; we're going to need to rely if yarn commit to get it right reliably"

That's not your experience? If not, great!

[piotr-oles]

Haha, you don't need emoji, don't worry - just type: message :) You have just few types to remember: feat, fix, chore, doc, perf, test :)
Adding BREAKING CHANGE is a little bit more tricky, but I'm sure you will not make it via phone 😄