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.

Sign up for GitHub

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

Skip dispatching messages if Turbo is not loaded #77

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Skip dispatching messages if Turbo is not loaded #77

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions app/assets/javascripts/hotwire_spark.js
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have edited the build output of rollup instead of the original source files. To properly adapt your patch you should add your changes to the originalmonitoring_channel.js async received() method and run npm run build to compile the files. Your patch should contain all changes (the build output and your original changes).

Original file line number Diff line number Diff line change
Expand Up @@ -1600,6 +1600,10 @@ var HotwireSpark = (function () {
document.body.setAttribute("data-hotwire-spark-ready", "");
},
async received(message) {
if (!window.Turbo) {
log(`Turbo not detected, skipping ${message.action}`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Turbo is only required if you're using the :replace method. The :morph variant should not require window.Turbo to be available. Could you check if your app is using the :replace or :morph variant?
I think this check should live in the html_replace_reloader.js to avoid skipping messages for all other reloaders.

return;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would hide errors for apps without Turbo and silently not update the page by default. I think a better approach here would be to always log a descriptive error with an explanation on how to avoid the error, e.g. [hotwire-spark] Tried to replace the page with Turbo, but Turbo is not available on window.Turbo or sth. similar :)

}
try {
await this.dispatch(message);
} catch (error) {
Expand Down