Add support for fork-ts-checker-webpack-plugin #160

[johnnyreilly]

Relates to #160

Hello!

Since it seemed to be fairly easy to plug this in I thought I'd have a go. It seems to work. You may not want this PR and that's fine. But I had some free time and so here you go. ❤️

Following my changes, if I put a linting issue in the code:

console.log('bad right?')

Here's what I see in the browser:

If I put a compilation error in the code:

this_is_an_actual_error

I see this in the browser:

Oh - GitHub won't let me copy that in. Not sure why. Anyway, it says:

C:/work/my-app/src/App.tsx
(24,1): Cannot find name 'this_is_an_actual_error'.

I wasn't sure exactly how you're supposed to test changes. I just created a react app using the existing mechanism, applied my changes over the top and then ran. There may be a "proper" way to test things. Happy to be instructed.

[johnnyreilly]

I have no idea how I killed the test with my change.

[DorianGrey]

That's a curious error:

../react-dev-utils/node_modules/http-parser-js/package.json:1
  1: {
     ^ Unexpected end of input

Occurs on TEST_SUITE=simple on both node 6 and 8. Do these execute normally if you run them in your local environment?

[johnnyreilly]

Weirdly running npm run build results in:

Cannot find "C:\source\create-react-app-typescript\packages\react-scripts\config\tsconfig.json" file. Please check webpack and ForkTsCheckerWebpackPlugin configuration.
Possible errors:
  - wrong `context` directory in webpack configuration (if `tsconfig` is not set or is a relative path in fork plugin configuration)
  - wrong `tsconfig` path in fork plugin configuration (should be a relative or absolute path)

I'm afraid I don't know how this ought to be fixed...

[wmonk]

@johnnyreilly you can see the better error now - think before it was just a Travis issue - but I have rerun and now you can see the actual error being an issue with your change. You should rebase against master branch to get latest changes as well.

[DorianGrey]

@johnnyreilly :

I'm afraid I don't know how this ought to be fixed...

In the current version of the commit, the config for ForkTsCheckerWebpackPlugin does not explicitly point to the tsconfig.json and tslint.json files. This might be a problem here, since the scripts are using several workarounds to support both ejected and non-ejected projects.
I'd suggest to explicitly list set the paths to the relevant config files and the directory to watch. You can use the paths helper for this:
https://github.com/wmonk/create-react-app-typescript/blob/master/packages/react-scripts/config/paths.js

So the resulting plugin config might look like this.

new ForkTsCheckerWebpackPlugin({
  checkSyntacticErrors: true,
  async: false,
  watch: paths.appSrc,
  tsconfig: paths.appTsConfig,
  tslint:  paths.appTsLint
}),

paths.appTsLint is not defined yet - just add it as required, similar to the way the paths for the tsconfig have been added.

[johnnyreilly]

Thanks folks - that's helpful. I might drop cache and thread loader given @DorianGrey comments on #160. Seems entirely reasonable

[johnnyreilly]

I hope I merged successfully there - never used the GitHub tools for that before...

[johnnyreilly]

hmmm ... the diff doesn't look right to me. I'll try and fix this up later (out of time for now)

[johnnyreilly]

all in all my merge appears to be a terrible mess! I think I may renew the fork and start again...