Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix not rebuilding files when rename event is emit #9689

Merged
merged 5 commits into from Nov 3, 2022

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.

None yet

2 participants