sass / dart-sass

The reference implementation of Sass, written in Dart.
https://sass-lang.com/dart-sass
MIT License
3.98k stars 357 forks source link

Watch/poll missing changes when files change #2440

Open hcker2000 opened 1 week ago

hcker2000 commented 1 week ago

To be more specific when switching git branches with sass --watch or sass --watch --poll it will miss the new @use lines inside the sass file that's supposed to be being watched.

For example if we add the following to branch new-fonts:

@use newfonts

Then switch back to the master branch it will still try to the @use line even though the file and line no longer exist.

nex3 commented 1 week ago

Unfortunately there's not much we can do about this. Sass relies on the operating system and intermediate libraries to notify it when files change on disk, and there's no way for it to tell when that stack is feeding us inaccurate information. You can always use the --poll option, which will be slower in general but more accurate since it just checks the filesystem directly instead of relying on OS notifications.

hcker2000 commented 1 week ago

It's also failing to pick up the changes when using --poll option. It seems like it's caching the sass file that's our entry point where all our @use statements are.

On Fri, Nov 22, 2024, 6:08 PM Natalie Weizenbaum @.***> wrote:

Unfortunately there's not much we can do about this. Sass relies on the operating system and intermediate libraries to notify it when files change on disk, and there's no way for it to tell when that stack is feeding us inaccurate information. You can always use the --poll option, which will be slower in general but more accurate since it just checks the filesystem directly instead of relying on OS notifications.

— Reply to this email directly, view it on GitHub https://github.com/sass/dart-sass/issues/2440#issuecomment-2495066757, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFZQXSQONWUMXUV2DAP6I32B62PNAVCNFSM6AAAAABSKGUUHGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIOJVGA3DMNZVG4 . You are receiving this because you authored the thread.Message ID: @.***>

nex3 commented 6 days ago

Okay, that could be an issue on our end. How have you installed Dart Sass? Can you provide a repo along with minimal steps to consistently reproduce the issue?

hcker2000 commented 6 days ago

I will work on getting a sample environment setup

On Fri, Nov 22, 2024, 8:44 PM Natalie Weizenbaum @.***> wrote:

Okay, that could be an issue on our end. How have you installed Dart Sass? Can you provide a repo along with minimal steps to consistently reproduce the issue?

— Reply to this email directly, view it on GitHub https://github.com/sass/dart-sass/issues/2440#issuecomment-2495199944, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFZQXUQ3BK3N2STDGZKKDL2B7MWBAVCNFSM6AAAAABSKGUUHGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIOJVGE4TSOJUGQ . You are receiving this because you authored the thread.Message ID: @.***>

hcker2000 commented 5 days ago

Ok this replicates the issue: https://github.com/hcker2000/dart-sass-debug

If you checkout v2 and then main and then v2 you will see what happens, it looks like the logic is almost inverted as i am seeing this when I switch back to main from v2:

body {
  background-color: #0f0;
}

body div {
  font-weight: 800;
}

/*# sourceMappingURL=data:application/json;charset=utf-8,%7B%22version%22:3,%22sourceRoot%22:%22%22,%22sources%22:%5B%22_child.sass%22,%22_other.sass%22%5D,%22names%22:%5B%5D,%22mappings%22:%22AAAA;EACI;;;ACAA;EACI%22,%22file%22:%22output.css%22%7D */

It should be:

body {
  background-color: #0f0;
}

/*# sourceMappingURL=data:application/json;charset=utf-8,%7B%22version%22:3,%22sourceRoot%22:%22%22,%22sources%22:%5B%22_child.sass%22%5D,%22names%22:%5B%5D,%22mappings%22:%22AAAA;EACI%22,%22file%22:%22output.css%22%7D */

If you add a blank line to main and then save it when the main branch is checked out it will re-compile and be fine.

nex3 commented 4 days ago

I can't reproduce. When I run sass --watch --poll src/main.sass:src/main.css and switch back and forth between the main and v2 branches, the file gets updated correctly after a second or so each time.

Note that nothing in this repo actually loads _other.sass, so body div { font-weight: 800 } is never emitted.

hcker2000 commented 4 days ago

Sorry I missed pushing a commit on the v2 branch. pull and try now. I do know that the console shows that it re-compiles but ends up not being correct.

nex3 commented 4 days ago

Okay, that lets me reproduce it. This is indeed our bug. Here's what's happening:

We do debounce events to avoid situations like this, but it looks like our debounce timeout is faster than the polling watcher's delay in reports so it's not working here. It's possible we should also add some additional mtime checks to avoid using stale cached ASTs.