Fork-ts-checker-webpack-plugin: Tests and app can get out of sync

[johnnyreilly]

First up - well done! I like this very much and thanks for https://github.com/TypeStrong/ts-loader/issues/537

However, I've found an issue when testing locally. I switched to use fork-ts-checker and deliberately broke the test code. I saw type errors and my tests failed. Great. Then I made some changes to my src code. The tests still failed. Great. Then I fixed the test code, the tests passed again. Awesome. Then I tweaked the src code and noticed the build was still reporting the type errors from when I deliberately broke the test. Nothing I do manages to let fork-ts-checker in on the news that the type error is gone. 😢

I'm guessing this is something to do with having my tests running in a separate process from my webpack build. My setup was based on this:

https://github.com/TypeStrong/ts-loader/tree/master/examples/webpack2-gulp-react-flux-babel-karma

with:

new ForkTsCheckerWebpackPlugin({
      watch: ['./src', './test'] // optional but improves performance (less stat calls)
})

Do you think if this an issue of my setup or is this a problem with fork-ts-checker? I'd be interested to see a setup as above tweaked to use fork-ts-checked in the correct manner assuming this is user error.

[piotr-oles]

Thanks for reporting :) Your config looks good, it's probably bug. I've tried to replay your scenario but it was working on my machine.
Please, send me your TypeScript version, tsconfig.json content and operating system version :)
Also please check if issue appear without watch option.

[johnnyreilly]

Here's a repro project (based on one of ts-loader's examples): https://github.com/johnnyreilly/fork-checker-repro

It has 2 commits; the initial setup and adding forktscheckerplugin so you can easily see what I've done. It's a little hacked together but should allow you to repro the issue mentioned above.

[piotr-oles]

I reproduced this error also on first commit so it looks like it's not because of this plugin.
Scenario:

1. put const x: number = '1'; at the end of the test/stores/GreetingStore.tests.ts
2. put const x: number = 1; at the end of the src/stores/GreetingStore.ts
3. remove const x: number = '1'; from test/stores/GreetingStore.tests.ts
4. remove const x: number = 1; from src/stores/GreetingStore.ts
   The console prints: error TS2322: Type '"1"' is not assignable to type 'number'. and error TS6133: 'x' is declared but never used. on test/stores/GreetingStore.tests.ts

These errors are reported by ts-loader (also in second commit). In second commit ts-loader should not report semantic errors like these (only emit errors), but there is typo in gulp/webpack.js:74, and karma.conf.js:8 - it should be transpileOnly instead of transpile ;)
So if you fix these two typos, everything should work.

[johnnyreilly]

Cool - I'll give it a try and report back!

[johnnyreilly]

Also - perhaps validating supplied options would be a good idea!

[johnnyreilly]

So this looks really nice! I have one remaining question: when developing I have the notifier plugin in play:

https://github.com/johnnyreilly/fork-checker-repro/blob/master/gulp/webpack.js#L60

  myDevConfig.plugins = myDevConfig.plugins.concat(
    new webpack.optimize.CommonsChunkPlugin({ name: 'vendor', filename: 'vendor.js' }),
    new WebpackNotifierPlugin({ title: 'Webpack build', excludeWarnings: true }), // ****** <- this
    new ForkTsCheckerWebpackPlugin({
      blockEmit: false,
      tslint: false,
      watch: ['./src', './test'] // optional but improves performance (less stat calls)
    })
  );

It pops toasts when the build breaks and when it's mended. How can this be made to play well with the fork-ts-checker?

[piotr-oles]

It should work with blockEmit: true option, but it's not good for development :) To make it work with non-blocking mode you can use event hooks provided by plugin and write your own:

var notifier = require('node-notifier');

function ForkTsCheckerNotifierWebpackPlugin() {}
module.exports = ForkTsCheckerNotifierWebpackPlugin;

ForkTsCheckerNotifierWebpackPlugin.prototype.apply = function(compiler) {
  compiler.plugin('fork-ts-checker-receive', function (diagnostics, lints) {
    var error = diagnostics.length ? diagnostics[0] : null;

    if (error) {
      notifier.notify({
        title: 'Webpack',
        message: error.getContent(), // see ./lib/NormalizedMessage for full api
      });
    }
  });
}

I didn't check this code, just write to show the idea :)

[johnnyreilly]

Perhaps I shall write the plugin!

[johnnyreilly]

I have a rough and ready plugin working. I'm going to do some more work on it and, if it seems worthwhile, I may publish it to npm. I may have some questions if that's okay?

[piotr-oles]

Yes, of course :)

[johnnyreilly]

Okay - I've got a first version written: https://github.com/johnnyreilly/fork-ts-checker-notifier-webpack-plugin

It's published to npm: https://www.npmjs.com/package/fork-ts-checker-notifier-webpack-plugin

🍾

There's one problem I've noted which could be:

1. User error
2. A bug in my notifier
3. A bug in fork-ts-checker-webpack-plugin.

I'll put up details in a while. Bedtime now I think. If you've any comments on the plugin then let me know.

[johnnyreilly]

Okay - I've updated the repro to use the new plugin: https://github.com/johnnyreilly/fork-checker-repro

I've deliberately introduced 2 bugs into the codebase; one into an src module (i.e. the actual app code) and one into a test module.

To see the strange behaviour follow these steps:

• npm install
• comment out the bugs in WhoToGreet.tsx (src module) and GreetingStore.tests.ts (test module)
• enter watch mode with npm run watch - the build should notify as succeeded.
• uncomment the bug in WhoToGreet.tsx, the build will notify as error. Recomment it, the build should notify as successful
• uncomment the bug in GreetingStore.tests.tsx, the build will not notify as error. Recomment it, the build will not notify as successful. This makes me 😞

I suspect it's something to do with the fact that I'm only using a single fork-ts-checker-webpack-plugin - just the one with the webpack build itself. Perhaps I should using one fork-ts-checker-webpack-plugin for the watched tests and another for karma as well?

[johnnyreilly]

Okay - just pushed one more commit that may resolve things. Just need to think about it and test it a littlle more. Will report back.... Any observations welcome

[johnnyreilly]

I've just published a new version of the plugin; it now handles lints as well as diagnostics. The repro repo is now working really nicely with fork-ts-checker-webpack-plugin. So much so that I'm planning to use it as a basis for another example in the ts-loader repo. Essentially a webpack2-gulp-react-babel-karma-fork-ts-checker example - this will be particularly useful to me in future I'm sure.

I'm planning to blog about both fork-ts-checker-webpack-plugin and fork-ts-checker-notifier-webpack-plugin. I hope that's okay with you?

[piotr-oles]

I'm glad to hear that :) Of course it will be my pleasure if you will write some article about this plugin. Please, send me your blog address ;)

[johnnyreilly]

It's https://blog.johnnyreilly.com/ - I'll be sure to mention you. Probably will post in a week or two. If you're on Twitter then let me know your handle and I'll copy you in when I tweet about it.

[piotr-oles]

I didn't have a Twitter account but I think it's a good time to sign up :) @OlesDev