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

feat: hot reload, fixes #82 #130

Merged
merged 10 commits into from
Aug 29, 2023
Merged

feat: hot reload, fixes #82 #130

merged 10 commits into from
Aug 29, 2023

Conversation

a-h
Copy link
Owner

@a-h a-h commented Aug 24, 2023

No description provided.

@joerdav
Copy link
Collaborator

joerdav commented Aug 25, 2023

Worth a look, github have a fork of the browser package imported in this PR that is actively maintained and updated:
https://github.com/cli/browser

@a-h
Copy link
Owner Author

a-h commented Aug 28, 2023

I'm getting fairly happy with this now. I've been doing some work to reduce the CPU utilisation, since I noticed it was using a lot of CPU while watching.

I added profiling, and noted that walking the directory every 250ms caused a lot of syscalls, and switched to WalkDir.

However, I think it's still too much. Next, I thought that I'd try out a phased approach:

  • On start, walk all directories and compile a list of files.
  • Every 250ms, check the existing set of found files for changes, and maybe do a backoff so that it will wait for up to 1000ms (not sure on the right value yet) to check files that haven't changed recently.
  • Every 1000ms-2000ms, re-walk all directories and update the list of found files.

@joerdav
Copy link
Collaborator

joerdav commented Aug 29, 2023

It would be more work to maintain, but you could use a separate technique for systems that support inotify rather than polling.

Or if you're willing to take on another dep:
https://github.com/fsnotify/fsnotify

@a-h
Copy link
Owner Author

a-h commented Aug 29, 2023

I tried out using fsnotify and friends, but couldn't get it to work well on my machine.

Here's an abandoned branch - https://github.com/a-h/templ/tree/hot_reload_watcher

I'm not sure I want to support the weird errors I'll get with a filesystem watcher implementation, since it requires me to have a machine for each OS and to test it out.

@a-h a-h merged commit 5e8644b into main Aug 29, 2023
6 checks passed
@a-h a-h deleted the hot_reload branch August 29, 2023 19:57
@a-h a-h mentioned this pull request Aug 29, 2023
a-h added a commit that referenced this pull request Sep 24, 2023
* wip: hot reload, need to work on client side element

* wip: get the proxy to reload the page

* refactor: switch to Github's fork of github.com/pkg/browser

* refactor: split up into smaller packages

* feat: add HTTP profile

* fix: ensure that commands are cancelled

* refactor: use the more efficient WalkDir method to reduce needless syscalls

* refactor: ensure cancellation works well

* feat: use backoff to reduce CPU utilisation

* docs: add new templ generate docs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants