Skip to content

Conversation

@natrys
Copy link
Contributor

@natrys natrys commented Oct 29, 2022

Presently when certain editors "atomically" change files, the CLI watcher fails to rebuild for them. This happens to me when using Vim, as by default it's behaving as if backupcopy is set to no. However the default value is auto which means Vim autodetects the system and chooses whichever strategy it deems optimal, so I am replicating the issue with an explicit config file:

set nobackup
set noswapfile
set backupcopy=no

Now when editing files, following inotify events are triggered:

MOVED_FROM	./index.html
MOVED_TO	./index.html~
CREATE	./index.html
MODIFY	./index.html
MODIFY	./index.html
MODIFY	./index.html
CLOSE_WRITE	./index.html
CLOSE_WRITE	./index.html
DELETE	./index.html~

Adding a console.log in the raw event handler, we can see chokider is emitting rename followed by change events:

DEBUG=1 strace -fe inotify_init,inotify_init1,inotify_add_watch,inotify_rm_watch node /home/natrys/dev/codebase/tailwindcss/lib/cli.js --config=tailwind.config.js --input=input.css --output=output.css --watch
strace: Process 10191 attached
strace: Process 10192 attached
strace: Process 10193 attached
strace: Process 10194 attached
strace: Process 10195 attached
strace: Process 10196 attached
strace: Process 10197 attached
strace: Process 10198 attached
strace: Process 10199 attached
strace: Process 10200 attached

Rebuilding...
Searching for config: 0.108ms
Module dependencies: 0.246ms
Loading config: 7.9ms
Creating context: 53.653ms
Resolving content paths: 1.513ms
Watch new files: 2.604ms
Reading content files: 3.054ms
Reading changed files: 1.328ms
Generate rules: 3.498ms
Build stylesheet: 0.396ms
Potential classes:  34
Active contexts:  0
Compiling CSS: 90.528ms
Recording PostCSS dependencies: 0.005ms
Watch new files: 0.396ms
[pid 10190] inotify_init1(IN_NONBLOCK|IN_CLOEXEC) = 22
[pid 10190] inotify_add_watch(22, "/home/natrys/dev/projects/debug/tailwind_watch/input.css", IN_MODIFY|IN_ATTRIB|IN_MOVED_FROM|IN_MOVED_TO|IN_CREATE|IN_DELETE|IN_DELETE_SELF|IN_MOVE_SELF) = 1
[pid 10190] inotify_add_watch(22, "/home/natrys/dev/projects/debug/tailwind_watch/tailwind.config.js", IN_MODIFY|IN_ATTRIB|IN_MOVED_FROM|IN_MOVED_TO|IN_CREATE|IN_DELETE|IN_DELETE_SELF|IN_MOVE_SELF) = 2
[pid 10190] inotify_add_watch(22, "/home/natrys/dev/projects/debug/tailwind_watch", IN_MODIFY|IN_ATTRIB|IN_MOVED_FROM|IN_MOVED_TO|IN_CREATE|IN_DELETE|IN_DELETE_SELF|IN_MOVE_SELF) = 3
[pid 10190] inotify_add_watch(22, "/home/natrys/dev/projects/debug/tailwind_watch/index.html", IN_MODIFY|IN_ATTRIB|IN_MOVED_FROM|IN_MOVED_TO|IN_CREATE|IN_DELETE|IN_DELETE_SELF|IN_MOVE_SELF) = 4
change triggered for output.css
change triggered for output.css

Done in 142ms.
rename triggered for index.html
rename triggered for index.html~
rename triggered for index.html
rename triggered for index.html
change triggered for index.html
change triggered for index.html
change triggered for index.html
rename triggered for index.html
rename triggered for index.html
rename triggered for index.html~
[pid 10190] inotify_rm_watch(22, 4)     = -1 EINVAL (Invalid argument)
[pid 10190] inotify_add_watch(22, "/home/natrys/dev/projects/debug/tailwind_watch/index.html", IN_MODIFY|IN_ATTRIB|IN_MOVED_FROM|IN_MOVED_TO|IN_CREATE|IN_DELETE|IN_DELETE_SELF|IN_MOVE_SELF) = 5

So the file change notification is picked by Tailwind, but no rebuild actually happens.

In my testing, when rename event handler is calling recordChangedFile from inside promise, then down the stack when rebuild is called, the chain promise is already at a pending state, and this condition is never fulfilled, so rebuild() is never called. I think might be the bug why rebuild doesn't occur in Visual Studio too as per #9667.

Additionally for atomic writes, the change is preceded by rename so a rebuild is already queued. It would end up happening twice, so a guard during the check could avoid that.

@natrys natrys changed the title Fix not rebuilding files when change event is emit Fix not rebuilding files when rename event is emit Oct 29, 2022
@thecrypticace thecrypticace self-assigned this Nov 2, 2022
@thecrypticace
Copy link
Contributor

thecrypticace commented Nov 2, 2022

There was another bug here in which rebuilds stop once an atomic renames of a temporary file failed. I'm not quote ready to merge this just yet as I need to do testing on Windows with Visual Studio, vim on linux and wsl2, etc…

@thecrypticace thecrypticace merged commit 0a4ae77 into tailwindlabs:master Nov 3, 2022
@thecrypticace
Copy link
Contributor

Thanks for the start on the PR! ✨

I made a handful of changes to organize and cleanup the watcher code, fix another bug I saw, and implement a kind of throttling to prevent excessive consecutive rebuilds because of how we schedule them.

This will be available in our insiders build which you can install via npm install tailwindcss@insiders

@natrys natrys deleted the inotify branch November 4, 2022 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants