Closed ayushn21 closed 6 months ago
jaredcwhite
commented
6 months ago Very cool @ayushn21! Seems snappy. I did notice that without the error handling on the client-side, the live reload doesn't pick up again when shutting down the server and then starting it up again. Would it be OK to commit a fix to your branch, or should I handle it separately?
ayushn21
commented
6 months ago I did notice that without the error handling on the client-side, the live reload doesn't pick up again when shutting down the server and then starting it up again.
That's a bit weird ... which browser are you using? I just tried on Firefox and it reconnects automatically when restarting the server, that's why I removed that code.
Feel free to commit to this branch :) (It's on my fork, let me know if you need write access or if the Bridgetown repo access carries over, not sure).
I'm still mucking about with this a bit. I'll leave a comment here when it's good to go from my end. I spent far too long last night playing around with a Queue system to trigger reloads from watcher.rb directly in code rather than writing to a file. I eventually realised that Puma runs in multiple processes so that kind of comms won't work .... felt like such a muppet when I realised that hahahaha!
jaredcwhite
commented
6 months ago OK, I pushed up changes. Not sure about Chromium browsers, but on Safari at least it doesn't attempt to reconnect automatically, so we still need the reconnection logic. I made it a bit smarter this time though…on the 25th failure it'll tap out and not try again. I also bumped the file checking frequency a tad so it's every half-second.
Checking index.html is a bit of a hack, and we'll need something smarter once I get my Fast Refresh feature working…since then we might not have a rebuilt index file unless we do a manual "touch". Maybe there's some other dot file we could use instead to indicate "time to refresh now!"
ayushn21
commented
6 months ago Checking index.html is a bit of a hack, and we'll need something smarter once I get my Fast Refresh feature working…since then we might not have a rebuilt index file unless we do a manual "touch". Maybe there's some other dot file we could use instead to indicate "time to refresh now!"
I played with the idea of the watcher writing a temporary file whose singular purpose is to trigger refreshes. We can use the Listen gem to watch that file and use it for refreshes. Any errors can be written into that file itself.
It was past 1am when I was trying to get this working though so I gave up and went to sleep 🤣
ayushn21
commented
6 months ago Hmmm, strange ... reconnection without the error listener works for me on Safari 17.2.1. Which version did you try on @jaredcwhite?
I'm happy to have the error handler in there but I'm just curious about this issue :). Here's what I did.
I didn't reload the page manually after restarting the server.
jaredcwhite
commented
6 months ago @ayushn21 I'll give it another try and see if I can provide more information about what was or was not working.
ayushn21
commented
6 months ago @jaredcwhite cool thanks, don't worry about it too much it's more about my curiosity than anything real :)
jaredcwhite
commented
6 months ago @ayushn21 right on. I can't wait to pair this up with #872…I have a feeling editing content files will feel wicked fast now. 🙌
This is a 🙋 feature or enhancement.
Summary
Rack 3 formalised bi-directional connections into the spec and implementations. This allows us to pass a proc instead of an array of strings in the Rack response.
Using this method, we can significantly simplify the code for live reloads.
Context
This was a result of my spontaneous deep dive into Ruby fibers, the Async gem, and by extension, Rack 3 and Falcon.
Background info:
https://youtu.be/yXyj9wlkJKM?si=prkP1nNCfA8WNBSU&t=1396
Caveats
This will only work in Rack 3. I think we should set that as a minimum requirement for BT2.0 unless there's a good reason not to?
Fixes https://github.com/bridgetownrb/bridgetown/issues/861