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

Improve reload experience when using with site generator such as cargo doc #83

Merged
merged 1 commit into from
Jun 23, 2024

Conversation

Pistonight
Copy link
Contributor

@Pistonight Pistonight commented Jun 4, 2024

Thanks for making this tool! I was looking into making something similar but it would take me way longer if I were to start from scratch... I found some issues when using this as a live-reload service for cargo doc. Please find the details and fix below

Problem Description

  1. When using a static site generator that nukes the output and regenerates it like cargo doc, the server can send update before the build is done, causing massive flickering and in the worst case, stuck on a 404 page.
  2. (If we pretend the above is fixed), location.reload() can still cause slight flickering because assets are not cached
  3. When server restarts, the client doesn't automatically reconnect

The issue(s) can be observed by running live-server target/doc and cargo watch -x doc side-by-side (requires cargo-watch)

Issue screenshot

bug

Fix Description

  1. Client: Probe the page using a query parameter ?reload and ensure it can load before triggering the reload. The server injects a meta tag if the page is a success response for reload (to distinguish it from 404 page)
  2. Client: Instead of calling location.reload(), the client:
    • refreshes the cached assets
    • reload the page inside an iframe
    • swap the head and body into the main document
      Because the assets are cached, the reload experience is smoother
  3. Client: Automatically try to reconnect after disconnecting
  4. CLI/Lib: There are probably some edge cases/special scripts that doesn't work with the soft reload approach, so I add hard_reload()/--hard to turn the optimization off and call location.reload() instead. This still ensures the page can be loaded so (1) is not an issue.

Testing

  • Updated test.rs
  • Tested soft and hard reload in Chrome, Edge and Firefox. Didn't test in Safari because I don't have a mac

Screenshots:

Hard Reload (notice the slight flicker)

hard_flicker

Hot Reload

hot2

Other

Not related to everything above, I also found an issue where links in the output of cargo doc were broken because ../ behaves differently when the path is <origin>/live_server vs. <origin>/live_server/ or <origin>/live_server/index.html.
so I added a check to redirect directories that don't end with slash. This might break scenarios that rely on this bug

@Pistonight Pistonight force-pushed the feature/better_reload branch from f6e77f8 to eb4c662 Compare June 4, 2024 21:37
@lomirus
Copy link
Owner

lomirus commented Jun 5, 2024

Thanks for your contribution. However I'm quite busy lately and can't spare extra time for reviewing currently. I will review it after I finish my work in about one or two week(s) :)

@Pistonight
Copy link
Contributor Author

I also just tested it on my laptop with 11th gen i5. It's pretty smooth for lower-end device as well :)

src/templates/websocket.js Outdated Show resolved Hide resolved
src/templates/websocket.js Outdated Show resolved Hide resolved
src/server.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
src/server.rs Outdated Show resolved Hide resolved
src/templates/websocket.js Outdated Show resolved Hide resolved
src/templates/websocket.js Outdated Show resolved Hide resolved
src/templates/websocket.js Outdated Show resolved Hide resolved
src/server.rs Show resolved Hide resolved
src/templates/websocket.js Outdated Show resolved Hide resolved
@Pistonight
Copy link
Contributor Author

Thanks for the review, I will update it soonish

@lomirus
Copy link
Owner

lomirus commented Jun 22, 2024

LGTM. Now just needs some adjustments to pass the checks.

@Pistonight Pistonight force-pushed the feature/better_reload branch from 168f47d to 96a99a8 Compare June 22, 2024 16:54
@Pistonight
Copy link
Contributor Author

Should be fixed now

@lomirus lomirus merged commit 4b0053f into lomirus:main Jun 23, 2024
1 check passed
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.

2 participants