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

Saving file in VSCode emitting two change events causing Trunk to build twice #238

Closed
lukechu10 opened this issue Aug 24, 2021 · 9 comments · Fixed by #350
Closed

Saving file in VSCode emitting two change events causing Trunk to build twice #238

lukechu10 opened this issue Aug 24, 2021 · 9 comments · Fixed by #350
Assignees
Labels
bug Something isn't working cli Trunk CLI application

Comments

@lukechu10
Copy link
Contributor

This is intended behavior in VSCode (microsoft/vscode#9419) because it relies on NodeJS fs.writeFile behavior (nodejs/node#6112). However, this means that Trunk will build the app twice.

For example, if you open the following workspace in gitpod.io and save src/main.rs, Trunk will build twice.
https://github.com/sycamore-rs/sycamore-trunk-gitpod-template

Happens on both windows and linux.
Does not occur in 0.12.1 but occurs in 0.13.0.

I believe this is due to notify being updated to 5.0.0-pre.11 (which does not seem to perform any debouncing? not really clear from the docs).

This can probably be fixed by debouncing notify events or downgrading to notify v4.

@thedodd
Copy link
Member

thedodd commented Aug 25, 2021

Good find. I would really prefer not to downgrade. There are options and event types that we can filter on within v5, so I would prefer that we do our best to make it work that way. Thanks for the report boss!

@thedodd thedodd added bug Something isn't working cli Trunk CLI application labels Aug 25, 2021
@lukechu10
Copy link
Contributor Author

I was trying to solve this problem and when logging the EventKind, I always get 2 Modify(Any)s for the same file.
Adding PreciseEvents doesn't seem to work on windows (I haven't tried on linux yet).

@lukechu10
Copy link
Contributor Author

The weirdest thing just happened: I can't reproduce this anymore lol

@lukechu10
Copy link
Contributor Author

Oh wait, I'm using the wrong version of trunk. Silly me

@lukechu10
Copy link
Contributor Author

Wait, I am using v0.13.1. Sorry for the confusion

@d4h0
Copy link

d4h0 commented Sep 9, 2021

This is especially a problem, if hooks are used that take some time.

This is the relevant PR where work is done to add debouncing back to notify. But it seems, it could take some time until this is released.

If someone has the same problem with hooks:

I'm using the following ugly work-around to run hooks only once:

[[hooks]]
stage = "build"
command = "sh"
command_arguments = [
    "-c",
    "if [ ! -f .hook-lock ]; then touch .hook-lock && $YOUR_HOOK; else rm .hook-lock; fi",
]

(Replace $YOUR_HOOK with your hook).

To make sure, that the lock file doesn't exist at startup, run trunk like this:

rm -f .hook-lock && trunk serve

Edit:

There is a disadvantage with the work-around above:

If you (like me) are generating CSS with your hook, the page will live-reload after the first build, and there will be no CSS available. After a second the second build finishes and the page reloads again with the generated CSS.

@dnaka91
Copy link
Contributor

dnaka91 commented Jan 12, 2022

I ran into this same problem recently, as I mainly use vscode. Always try to run the pipelines twice, and it seems Trunk is quite unstable being run that often in succession. Sometimes get random build errors because some generated files are missing and such.

Found a WIP PR at the notify repo, and the maintainer seems to just be busy with other things at the moment. Good thing is, they definitely want that feature back in. You can see at the bottom that they recommend just sticking with notify v4 until this PR is finished.

So I would say it may be the best for now to downgrade to notify v4 and upgrade to v5 later again, once the full (non-pre) version is out.

@maxcountryman
Copy link

As documented elsewhere, this simply doesn't work with vim at the moment. Upstream fixes are delayed and the prescription is to use v4 for the time being.

@dnaka91
Copy link
Contributor

dnaka91 commented Apr 10, 2022

Yes, the best solution really seems to be to go back to v4. Had a look at that a while back, but then stopped working on it as it turned out to be quite a big change.

The problem is not just that the API is very different, but that we had many changes around that code in the meantime.

Hopefully I'll be able to have another look at it soon again, but it's probably taking a while to get the downgrade implemented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working cli Trunk CLI application
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants