Fix sass watching 100,000+ files in dev #73
Conversation
| @@ -1,4 +1,4 @@ | |||
| @import '@deephaven/components/scss/custom.scss'; | |||
| @import '../../components/scss/custom.scss'; | |||
There was a problem hiding this comment.
I don't really like having to do a relative dir for components like this - it makes it more fragile, if we move one of these scss files or one of those folders we break the chain.
Did you take a look at the sass watch source at all to see if there's any way to restrict the types of files it actually watches? Would love to have an upstream fix (or issue) filed as well.
There was a problem hiding this comment.
I found where they have it hardcoded to ignore files that aren't sass... https://github.com/sass/dart-sass/blob/f6819060ec54358e318fafa6395238975030834c/lib/src/executable/watch.dart#L129
if (extension != '.sass' && extension != '.scss' && extension != '.css') {
continue;
}
There was a problem hiding this comment.
I think I'd be using chokidar, so this watch? https://github.com/sass/dart-sass/blob/f6819060ec54358e318fafa6395238975030834c/lib/src/io/node.dart#L218
I wonder if it would be better with globbing (specifying **/*.scss or something). They mention being judicious about dirs you're watching as it may recursively watch a lot: https://github.com/paulmillr/chokidar#how
They've also got another watch impl here but I don't think I'm hitting this one: https://github.com/sass/dart-sass/blob/f6819060ec54358e318fafa6395238975030834c/lib/src/io/vm.dart#L90
There's also a couple relevant issues that we could track/add to:
sass/dart-sass#1292
sass/sass#2739 (which speaks about a fix not requiring modifying load-paths)
There was a problem hiding this comment.
These could be kept for now. There's 3 or 4 packages which need to watch their local node_modules and add a few thousand watchers each.
Could also specifically watch node_modules/@deephaven/components/scss but then the imports are less clear since it would just be import 'custom.scss'
I tried watching globs and it didn't work. It's also really annoying because from the source it looks like it should detect only sass files first and then only pass those to watch, but that isn't working on Linux it seems.
| @@ -1,7 +1,7 @@ | |||
| // Styling overrides for bootstrap | |||
|
|
|||
| //Make bootstrap functions available for use in overrides | |||
| @import 'bootstrap/scss/_functions.scss'; | |||
| @import '../../../node_modules/bootstrap/scss/_functions.scss'; | |||
There was a problem hiding this comment.
I'd be okay keeping this one relative (as it's just in one spot), but add a comment why we need relative instead of doing the normal thing with load-path.
Would doing load-path=node_modules in the other projects be enough? Then just need the one relative import here. Would like a comment, and possibly a link to an upstream sass issue.
Also you say this doesn't happen on Windows or Mac? What's different with the watches?
There was a problem hiding this comment.
Even having glob paths to specify would help: sass/dart-sass#616
I don't know, the watch seems limiting!
There was a problem hiding this comment.
load-path=node_modules would work for the others, it just adds 1-2k watchers for 3 different packages. Which isn't that big of a deal compared to it watching 100,000+.
I'm not sure what WSL was doing since it should be running the Linux kernel, but my guess is the FS is a bit different from a normal Linux FS and doesn't use inotify. Mac uses FSEvents through chokidar
There was a problem hiding this comment.
Okay let me know if you want me to test that out
Fixes #60
This only seems to affect Linux (not WSL or Mac). The others use a different file watching protocol through chokidar. Not sure why WSL does since it should be Linux, but I had 0 inotify watches when trying it out on WSL.