Fix: immediate recompilation #256
Conversation
… otherwise the kill signal will not be received by the child reliably.
src/compiler.ts
Outdated
| const starTime = new Date().getTime() | ||
|
|
||
| const fileName = params.compile | ||
| const code = params.code || fs.readFileSync(fileName, 'utf-8') |
There was a problem hiding this comment.
We use the passed params.code, if available, to avoid reading the file twice. We also do it much later to avoid reading it if it isn’t necessary.
| @@ -81,6 +81,10 @@ If you have an issue, please create one. But, before: | |||
| - try to run it with `--debug` flag and see the output | |||
| - try to make create repro example | |||
|
|
|||
| ## Building | |||
|
|
|||
| Run `tsc --build tsconfig.build.json` to transpile this project. | |||
There was a problem hiding this comment.
… just because I had to look and experiment for a while to figure out how to compile this project.
|
@wclr what do you think about this PR? Looks like 2 nice improvements without a lot of code changes. |
|
+1 here. We are facing the same issue of high CPU usage occasionally as @rluba mentioned. Would be great if we could get this fixed ASAP. |
src/compiler.ts
Outdated
| const starTime = new Date().getTime() | ||
|
|
||
| const fileName = params.compile | ||
| const code = params.code || fs.readFileSync(fileName, 'utf-8') |
There was a problem hiding this comment.
Don't think this line even needs to still be here because the 'code' local variable is not actually used.
There was a problem hiding this comment.
Removed the line as requested because the code was actually not used
|
@wclr Any things we need to fix which prevents this PR from being merged? |
|
BTW we've been using a version with this fix since August and initially it correctly restarted every time you made a change. But recently it's started not restarting sometimes when you make changes in quick succession (but still better than without this fix). Maybe related to a mac os update or node version update is my wild guess. Would recommend merging this in still since it at least improves behavior a little and has not broken anything after 2 months of using it. |
|
@wclr any progress on this PR? What is missing? |
|
It's quite painful and can bring a lot of confusion for big projects with long compilation time (like one I work in). |
We noticed that restarting the process upon changes did not work reliably if the process was still in the middle of compilation when the change occurred.
In that situation it seems like disconnecting from the child before sending the
SIGTERMcauses the child process to freeze. On my machine (MacOS 10.15, node.js 14.16.0) the child kept spinning at 100% CPU load, but did not react toSIGTERM– neither the one sent byts-node-devnor any signal sent manually.Not disconnecting from the child before killing it resolves the issue. It has the added advantage of showing why restarting may take longer than expected – you still receive the old child’s log output and see, eg., when the old compilation finally finishes.
While investigating the cause of the hang, I also noticed that
ts-node-devread the content of every changed file twice (and much earlier than needed). This PR also fixes that.