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

Support for page refreshes and broadcasting #499

Merged
merged 14 commits into from
Nov 23, 2023
Merged

Support for page refreshes and broadcasting #499

merged 14 commits into from
Nov 23, 2023

Conversation

afcapel
Copy link
Contributor

@afcapel afcapel commented Oct 5, 2023

This PR is the Rails companion for hotwired/turbo#1019

Helper to configure page refresh directives

turbo_refreshes_with scroll method: :morph, scroll: :preserve

Helpers to broadcast page refresh actions

This adds new Active Record helpers to broadcast page refreshes from models:

class Board
  broadcast_refreshes
end

This works great in hierarchical structures, where the child record touches parent records automatically to invalidate the cache:

class Column
  belongs_to :board, touch: true # +Board+ will trigger a page refresh on column changes
end

You can also specify the streamable declaratively:

class Column
  belongs_to :board
  broadcast_refreshes_to :board
end

There are also instance-level companion methods to broadcast page refreshes:

  • broadcast_refresh_later
  • broadcast_refresh_later_to(*streamables)

System to suppress stream broadcasts

This PR introduces a new mechanism to suppress the broadcasting of turbo streams for arbitrary blocks of code:

Recording.suppressing_turbo_broadcasts do
...
end

System to debounce broadcasted page refresh jobs

When the broadcasting page refreshes, the system will automatically debounce multiple calls in a row to only broadcast the last one. This is meant for scenarios where you process records in mass. Because of the nature of such signals, it makes no sense to broadcast them repeatedly and individually.

The debounce implementation uses concurrent-ruby and thread variables under the hood.

@afcapel afcapel mentioned this pull request Oct 5, 2023
2 tasks
@jvillarejo
Copy link

jvillarejo commented Oct 14, 2023

Should the debouncer implementation using concurrent-ruby be part of ActiveSupport or be part of this gem ?

Also, is the debounce for multiple calls useful only for page refreshes or for the other broadcasting methods too?
The broadcast_replace_to might be a good candidate.

@northeastprince
Copy link
Contributor

I would introduce the suppression system in a separate PR.

This PR is the Rails companion for the Turbo changes to add page
refreshes.

```ruby
turbo_refreshes_with scroll method: :morph, scroll: :preserve
```

This adds new Active Record helpers to broadcast page refreshes from
models:

```ruby
class Board
  broadcast_refreshes
end
```

This works great in hierarchical structures, where child record touch
parent records automatically to invalidate cache:

```ruby
class Column
  belongs_to :board, touch: true # +Board+ will trigger a page refresh
  on column changes
end
```

You can also specify the streamable declaratively:

```ruby
class Column
  belongs_to :board
  broadcast_refreshes_to :board
end
```

There are also instance-level companion methods to broadcast page
refreshes:

- `broadcast_refresh_later`
- `broadcast_refresh_later_to(*streamables)`

This PR introduces a new mechanism to suppress broadcasting of turbo
treams for arbitrary blocks of code:

```ruby
Recording.suppressing_turbo_broadcasts do
...
end
```

When broadcasting page refreshes, the system will automatically debounce
multiple calls in a row to only broadcast the last one. This is meant
for scenarios where you process records in mass. Because of the nature
of such signals, it makes no sense to broadcast them repeatedly and
individually.
end

def suppressed_turbo_broadcasts?
suppressed_turbo_broadcasts
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
suppressed_turbo_broadcasts
!!suppressed_turbo_broadcasts

A page refresh stream signals the need to reload the page. It's a global action
that makes sense to aggregate when multiple signals are generated in a short period
of time for a given streamable.

This implementation is based on creating a thread-level debouncer associated to
the set of streamables. The debouncer is implemented using concurrent-ruby's
scheduled tasks.
Comment on lines +146 to +147
def broadcasts_refreshes
after_commit -> { broadcast_refresh_later }
Copy link
Contributor

@tonysm tonysm Nov 6, 2023

Choose a reason for hiding this comment

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

So, this will only send broadcast refreshes to the current model's channel, right? Shouldn't it be closer to the broadcasts method and accept the stream argument or default to the model's plural name when creating the model (see #295)? Otherwise, it will send a broadcast to a channel that no one will ever be listening on (because the model is being created), right?

Suggested change
def broadcasts_refreshes
after_commit -> { broadcast_refresh_later }
def broadcasts_refreshes(stream = model_name.plural)
after_create_commit -> { broadcast_refresh_later_to(stream) }
after_update_commit -> { broadcast_refresh_later }
after_destroy_commit -> { broadcast_refresh }

Copy link
Contributor

Choose a reason for hiding this comment

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

Sent a PR: #521

jorgemanrubia and others added 8 commits November 13, 2023 09:36
This can happen when the Debouncer has finished its work and we clear it
from the current thread. The next call to refresh_debouncer_for creates
a new Debouncer, but it doesn't have a scheduled_task yet.

We only use the wait method in tests, to ensure that the debounces has
finished its work. But if the scheduled_task is nil, we know that the
debouncer has already finished its work.
@jon-sully
Copy link

jon-sully commented Dec 24, 2023

Just out of curiosity, why go with the _later approach for the broadcasts with refreshes? @afcapel raised a key point to my (admittedly "duh") question the other month in that we don't have a broadcast_remove_later method since broadcasting a 'remove' action requires no markup, and thus no background processing to save markup-time is necessary. Wouldn't this be the same, since there's no real markup, just a single (constant) html tag?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

6 participants