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

3 reload modes: hard, soft, css-only. Reload on error pages. #34

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

elik-ru
Copy link

@elik-ru elik-ru commented Feb 19, 2023

This PR introduce 2 major changes:

  1. 3 different reload modes (css, soft, hard)
  2. Auto-reload on error pages.

Reload modes:
Depending on what changed we can do different reload types.

  1. When css files are changed we can inject new CSS into current page. This is especially cool when you do some actions on page (open dropdown, collapses, some ajax request etc), which will be reset after a page reload. This requires external css build tools like dart-sass, which will automatically rebuild css files.
  2. When non-js files are changed we can do fast reload using Turbo.visit()
  3. When js files are changed we must do a full page reload. If we do Turbolinks.visit(), it will detect that js files are changed and do a full reload anyway, but this will result in double page load.

Auto-reload on error pages: I have added a middleware which injects livereload code into error pages.

To be honest, i did this couple of month ago and did not have time to submit a PR, and now looking at the code it looks like an alien wrote it. But if you think i need to improve something in this PR, i'll try to do this.

@kirillplatonov
Copy link
Owner

@elik-ru thanks for the PR 🙌 I will review and test on the weekend

@LEstradioto
Copy link

Nice PR. Tested at my end, the only thing that didnt work out was the default :css mode, my setup is jsbundling + cssbundling + talwindcss.

It fails silently here because my stylesheet tag do not contain a stylesheet data attribute, as your code is expecting. My link is:

<link rel="stylesheet" href="/assets/application-{{big-hash-here}}.css" data-turbo-track="reload">

I could fix it adding the data attribute manually, like this:

# application.html.erb
<%= stylesheet_link_tag "application", "data-turbo-track": "reload", "data-stylesheet": "application.css" %>

Is this expected?

@elik-ru
Copy link
Author

elik-ru commented Feb 24, 2023

@LEstradioto Thanks for your feedback!
Yes, this is kinda expected, because actual filename differs form digested url, and digested url changes when file changes, there is no easy way to match actual filename and corresponding <script> tag. So i simply added a data attribute. May be there is a way to patch stylesheet_link_tag and make it add data attribute automatically, but given that we have multiple options (sprockets of various versions, Propshaft etc), i think it does not worth it. It's easy to add attribute manually, we just need to document it properly.

@elik-ru
Copy link
Author

elik-ru commented Mar 15, 2023

Hi @kirillplatonov! How is it going? Can i help you in somehow to make it easier to review?

@kirillplatonov
Copy link
Owner

@elik-ru could you please extract reloading on error pages into separate PR? It would be easier to review and merge this way

@elik-ru
Copy link
Author

elik-ru commented Mar 18, 2023

@kirillplatonov There 2 commits for 2 features, but seems some improvements leaked from one to the other.
Ok, i'll prepare 2 separate PRs

@elik-ru
Copy link
Author

elik-ru commented Nov 22, 2023

Continuation: #46

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.

None yet

3 participants