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

Add debounce to fsnotify #1882

Merged
merged 2 commits into from
Jan 2, 2024
Merged

Add debounce to fsnotify #1882

merged 2 commits into from
Jan 2, 2024

Conversation

mpscholten
Copy link
Member

Fixes #1809

@mpscholten mpscholten requested a review from amitaibu January 1, 2024 14:15
ihp.cabal Outdated
@@ -105,6 +105,7 @@ common shared-properties
, with-utf8
, ihp-hsx
, ihp-postgresql-simple-extra
, auto-update
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this related to the debounce?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes the Control.Debounce module is part of that package

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add a comment to indicate it?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, added 👍

Copy link
Collaborator

@amitaibu amitaibu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Locally I see the "modules loaded" appears now only twice rather than three times, when I switch git branches.

image

Not sure if we can reduce to a single load?

ihp.cabal Outdated
@@ -105,6 +105,7 @@ common shared-properties
, with-utf8
, ihp-hsx
, ihp-postgresql-simple-extra
, auto-update
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add a comment to indicate it?

@mpscholten
Copy link
Member Author

Not sure if we can reduce to a single load?

Not possible without adding latency. We could use https://hackage.haskell.org/package/auto-update-0.1.6/docs/Control-Debounce.html#v:trailingEdge to first cool down and then trigger the reloads. The issue is that this adds latency to every normal single file change

@mpscholten mpscholten merged commit 47826ea into master Jan 2, 2024
1 of 2 checks passed
@mpscholten mpscholten deleted the filewatcher-debounce branch January 2, 2024 10:20
@amitaibu
Copy link
Collaborator

amitaibu commented Jan 2, 2024

Yeah, I was just reading about the trailingEdge, and it seems like a no go.

Thanks 🙏

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.

Re-add debounce to fsnotify
2 participants