TypeStrong / tsify

Browserify plugin for compiling TypeScript
344 stars 75 forks source link

Fix watchify stops listening after compilation errors #245

Closed jeremija closed 5 years ago

jeremija commented 5 years ago

Hi there, I noticed an issue with watchify where it would stop listening to a file after a syntax error. The only way to fix it other then restarting it would be to edit another watched file in the project and save it. I noticed that next() is never called in the flush function when there is an error. I added the call to next() before returning and now it seems to work.

I've also included a video of a simple TypeScript project to help illustrate what's going on. If required, I can upload that project to GitHub for further inspection.

test

cartant commented 5 years ago

Thanks. It's been a long time since I've looked at this project. It'll take me a while to remember what's what. I'll take a look at this later today.

jeremija commented 5 years ago

Thanks, @cartant! FWIW, I am unsure if I should've removed expectNoOutput from the test cases, but I suspect the only reason why there was no output was because the callback had never been called. When I run my local test with browserify, it will still exit with an error code, and there will be no output file generated if there is a syntax error. With watchify, the output will be a single line (which IIRC is similar to babelify):

console.error("TypeScript error: src/index.ts(2,20): Error TS1005: ',' expected.");

Upon further inspection, looks like through2 uses Transform from the readable-stream package. The docs for readable-stream say:

This package is a mirror of the streams implementations in Node.js.

Full documentation may be found on the Node.js website.

The official docs for transform._flush() say the following:

Within the transform._flush() implementation, the readable.push() method may be called zero or more times, as appropriate. The callback function must be called when the flush operation is complete.

cartant commented 5 years ago

FYI, things are busier than I'd anticipated, at work, so it's going to take me a little longer to look into your change. Tomorrow. Probably.

If you've not heard from me within a couple of days, please ping me to remind me.