-
Notifications
You must be signed in to change notification settings - Fork 125
Conversation
Codecov Report
@@ Coverage Diff @@
## master #167 +/- ##
=======================================
Coverage 94.66% 94.66%
=======================================
Files 22 22
Lines 656 656
=======================================
Hits 621 621
Misses 35 35 Continue to review full report at Codecov.
|
So responsive! Haha, thank you, I'll try it out as soon as I can. |
Hmm, I'm seeing something strange where the fork-ts-checker plugin isn't despawning its services. Is this intended? The example fails to run because the process is never killed. Here's a snippet that shows the remaining (zombie) processes:
I have to manually kill them to get this to clean up. |
@pietr-oles can you comment on this please? |
Let's try that again 😄 @piotr-oles would you be able to comment on this please? It sounds like there might be an issue with fork-ts-checker plugin despawning its services... |
This works great except with dll plugin. It builds the dll file, but the process does not exit.
|
Thanks @dziamid - that's good to know. Hopefully @piotr-oles can advise. I may open an issue on fork checker to track. |
I'm working on it, will let you know :) |
The problem with fork-ts-checker-webpack-plugin was misleading configuration. I've released v0.2.0 with some improvements you can check in changelog. The main change is removing |
I have verified that it works once we upgrade to 0.2.0 - merging this! Thanks all. |
The README now indicates TypeScript compatibility - thank you all for your work. |
I've simplified the example in the PR and removed api bits that are no longer part of fork-ts-checker. Hope that helps! |
"author": "John Reilly <[email protected]> ", | ||
"license": "ISC", | ||
"devDependencies": { | ||
"fork-ts-checker-webpack-plugin": "^0.1.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, change it to ^2.0.0 :)
}, | ||
plugins: [ | ||
new ForkTsCheckerWebpackPlugin({ | ||
tslint: false, // disable tslint support |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is unnecessary now - it's disabled by default
tslint: false, // disable tslint support | ||
watch: './src', // optional but improves performance (less stat calls) | ||
workers: ForkTsCheckerWebpackPlugin.TWO_CPUS_FREE, // use multi-process mode, leave 2 cpu's free for builder and system | ||
blockEmit: process.env.NODE_ENV === 'production' // for production make it synchronous |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please, remove this - this is no longer useful - it's computed automatically
entry: './src/index.ts', | ||
output: { | ||
path: path.resolve(__dirname, 'dist'), | ||
filename: '[name].raw.js' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should be:
path: path.resolve(__dirname, 'dist--raw'),
filename: '[name].js'
:)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oooh you might be right!
watch: './src', // optional but improves performance (less stat calls) | ||
workers: ForkTsCheckerWebpackPlugin.TWO_CPUS_FREE, // use multi-process mode, leave 2 cpu's free for builder and system | ||
blockEmit: process.env.NODE_ENV === 'production' // for production make it synchronous | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above - tslint and blockEmit is no longer needed :)
I've forgot to submit revew yesterday, it was pending. Never mind, it's not up to date :) |
This should resolve #164
I'm afraid I can't run the test as I'm on Windows. I've done my best to guess how it should work - may need your help there if it doesn't run. Fingers crossed!