De-simplify watcher

[dfreeman]

The change in #196 was nice in that it simplified our code, but unfortunately it wasn't a faithful recreation of how tsc's own watcher works. It turns out we're forcing tsc to do extra work by treating every file change as a directory-level action, and we're also running into an inconsistency in how tsc deals with setting up directory watches depending on the name of the folder the project is in.

This should fix #204 and fix #221. It also supersedes #215. I've stress tested it a bit with the type-generation branch, and I believe between the slightly different implementation here and the change in #218, the problem that #196 was originally addressing is no longer an issue. Scratch that. Investigation underway.

I'd love to confirm with at least one of @pixelhandler and/or @Bouke that this solves the rebuild issues they were having before merging.

[pixelhandler]

@dfreeman yes this branch fixes the rebuild issue I reported on #204. Just ran this branch with my app and changes to .ts files resulted in successful rebuilds while running ember b -w. Thanks for working on this :)

[Bouke]

@dfreeman this branch also resolves the issue I reported in #221. I also tested with ember build --watch in side my directory containing dots. Thank you for putting in the energy to resolve our issues!

[Bouke]

Removing a file gives the following errors, should those be covered by this?

file deleted routes\bouke.ts
error TS6053: File 'C:/My.Project.Name/app/routes/bouke.ts' not found.

Build successful (4166ms)



Slowest Nodes (totalTime => 5% )              | Total (avg)
----------------------------------------------+---------------------
Funnel (24)                                   | 743ms (30 ms)
LessCompiler (1)                              | 524ms
broccoli-persistent-filter:SimpleReplace (3)  | 438ms (146 ms)
Funnel: Addon JS (19)                         | 372ms (19 ms)
broccoli-persistent-filter:Babel > [Ba... (1) | 295ms

file changed routes\about.ts
error TS6053: File 'C:/My.Project.Name/app/routes/bouke.ts' not found.

Build successful (2928ms)

And after adding the same file again, the build is still hanging.

Adding a new file from a new watched build, gives the following error:

Build successful (88718ms)



Slowest Nodes (totalTime => 5% )              | Total (avg)
----------------------------------------------+---------------------
Babel (33)                                    | 36032ms (1091 ms)
LessCompiler (1)                              | 14521ms
SimpleReplace (3)                             | 12561ms (4187 ms)

EPERM: operation not permitted, lstat 'C:\My.Project.Name\app\routes\bouke3.ts'


Stack Trace and Error Report: C:\Users\Bouke\AppData\Local\Temp/error.dump.018cba7791c06d3789a076b620cb9bda.log
Cannot stop: not the current node.
Cannot stop: not the current node.
Cannot stop: not the current node.
Cannot stop: not the current node.
file added routes\bouke3.ts
Cannot stop: not the current node.
Cannot stop: not the current node.
Build Canceled

cannot build this builder, as it has been previously canceled


Stack Trace and Error Report: C:\Users\Bouke\AppData\Local\Temp/error.dump.cbb22e5ad9d3b42c70b59c154401c1a7.log
file added routes\bouke3.ts
cannot build this builder, as it has been previously canceled


Stack Trace and Error Report: C:\Users\Bouke\AppData\Local\Temp/error.dump.3e288bea1831e096ea4ae4f4dd759573.log

Also something else I've noticed is that Sublime doesn't show the file additions/deletions when the watched build is running. Could Windows be running into some limit here?

[dfreeman]

Alright, an update on this — even with the change I originally made here, tsc's own weirdness with directories that have . in them was still biting us. I've updated this so that the default blueprint adds **/* to the end of the paths in include, which gets tsc to stop thinking dir.name is somehow a glob on its own. Between that and a fix for a casing mismatch between tsc and chokidar, this should work nicely across the board now. I've verified locally and in a Windows VM that that seems to be the case.

Also something else I've noticed is that Sublime doesn't show the file additions/deletions when the watched build is running. Could Windows be running into some limit here?

I couldn't say for certain, but historically (in my own experience at least), Sublime has never been great with filesystem events. I'm not sure if there's a near-term fix, but I'd expect this to improve when Broccoli 2 lands in ember-cli, which will allow the tmp directory to move out of the project and into system temp.

[Bouke]

I changed my tsconfig.json to include **/*, and after that it all appears to work great! I did a few changes and file additions / deletions / renames and the build runs every time. There's an occasional error message printed, but it doesn't appear to affect the build itself:

Cannot stop: not the current node.
Cannot stop: not the current node.
file changed router.ts
Build Canceled

file changed router.ts
file changed router.ts
cannot build this builder, as it has been previously canceled


Stack Trace and Error Report: C:\Users\Bouke\AppData\Local\Temp/error.dump.701ac305e3eff3f67a29ba374793601e.log

Build successful (7951ms)

file deleted routes\about.ts
Build Error (EslintValidationFilter)

ENOENT: no such file or directory, stat 'C:\My.Project.Name\tmp\funnel-output_path-O6j5sLVx.tmp\routes\about2.ts'


Stack Trace and Error Report: C:\Users\Bouke\AppData\Local\Temp/error.dump.ee89fefd88ae82250d9b6d67a9568320.log
file deleted routes\about2.ts
file added routes\about.ts

Build successful (6237ms)

I couldn't say for certain, but historically (in my own experience at least), Sublime has never been great with filesystem events. I'm not sure if there's a near-term fix, but I'd expect this to improve when Broccoli 2 lands in ember-cli, which will allow the tmp directory to move out of the project and into system temp.

Ok great. It was just an observation, not sure if it has anything to do with this project. Thanks, I'll keep an eye out for those improvements in ember / brocolli. 👍

[dfreeman]

@Bouke In that first trace, had you canceled the build, or do you get those Cannot stop: not the current node errors just during active watched builds? For the second, it looks like that error is coming from ESLint, but it's certainly interesting that it's happening when a .ts file changes.

In both cases (and just in general), if you're able to either include the contents of the Stack Trace and Error Report file it references in the output as part of the issue or DM them to me on Slack, that can provide us a lot more information to go on.

Regardless, I'm glad things seem to be working for you! 🎉

[Bouke]

@dfreeman yes both these errors occurred in the same watched build, when adding/removing/deleting some files. Probably the timing of these file operations was just right to trigger those errors. The build did continue and no problems seem to have occurred. We're currently working off this branch, and it all seems to be fine thus far.

[chriskrycho]

I'm going to do some local testing and then presumably merge this by the end of the week! Sorry for the delays; I've been much occupied by my primary tasks at work.

[pixelhandler]

@chriskrycho let me know if you'd like me to test anything here.

[pixelhandler]

@chriskrycho I tested out our app, using, ember s, ember b -w, ember t -s, used the generator to create a component, modified existing typescript files, changed .js file to .ts file. Everything worked as expected, rebuilds fired off etc. This is great 👍