-
-
Notifications
You must be signed in to change notification settings - Fork 115
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
Use rack 3 proc responses for SSE live reload #858
Conversation
e127064
to
0a2ac2a
Compare
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? |
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 |
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 |
I played with the idea of the It was past 1am when I was trying to get this working though so I gave up and went to sleep 🤣 |
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. |
@ayushn21 I'll give it another try and see if I can provide more information about what was or was not working. |
@jaredcwhite cool thanks, don't worry about it too much it's more about my curiosity than anything real :) |
54027db
to
6f7c44d
Compare
6f7c44d
to
e4196de
Compare
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 #861