-
Notifications
You must be signed in to change notification settings - Fork 688
feat: add Poll component to standardize polling behavior #1556
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
Conversation
- Centralizes interval-based logic in reusable Vue component - Reduces polling frequency to hourly when window not visible - Improves code maintainability and reduces redundancy
- Centralizes interval-based logic in reusable Vue component - Reduces polling frequency to hourly when window not visible - Improves code maintainability and reduces redundancy
- Centralizes interval-based logic in reusable Vue component - Reduces polling frequency to hourly when window not visible - Improves code maintainability and reduces redundancy
- Centralizes interval-based logic in reusable Vue component - Reduces polling frequency to hourly when window not visible - Improves code maintainability and reduces redundancy
resources/js/components/Poll.vue
Outdated
@@ -0,0 +1,111 @@ | |||
<script type="text/ecmascript-6"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rewrite this as a plain javascript function and not a vue component. Now every polling function needs an associated html element in the DOM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a renderless component, so it does not create any DOM elements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You now specifically need to add <poll @poll="refreshBatchesPeriodically" />
in the code everywhere instead of a plain javascript solution like myCustomIntervalFunctionThatPausesInBackground(() => ..., 2000)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about the Lifecycle hooks?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes you'd still need the cleanup process; one such implementation could look like this:
import VisibilityAwareInterval from './VisibilityAwareInterval';
data() {
return {
this.my_interval: undefined,
};
},
mounted() {
// Create some visibility based interval:
this.my_interval = new VisibilityAwareInterval(cb, 2000);
},
beforeUnmount() {
// Call the `clear` fn in the custom class (optionally use `?.` here in
// case the `my_interval` is already cleared by other code):
this.my_interval?.clear();
}
The VisibilityAwareInterval
class will mostly contain the same code you have now, but instead it is a javascript class that exposes a few functions like start()/stop()/clear()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @bert-w for the follow-up.
I don't see any advantage to that approach since the existing code before my PR already handles it similarly.
With this PR, the implementation becomes much simpler:
<poll @poll="myCallback" />
The goal is to centralize polling logic and make it effortless to use by abstracting all complexity into this renderless component - similar to how wire:poll
works in Livewire. This way, developers don't need to worry about cleanup, initialization, or other implementation details.
- Centralizes interval-based logic in reusable Vue component - Reduces polling frequency to hourly when window not visible - Improves code maintainability and reduces redundancy
This significantly reduces the bandwidth usage when the browser tab is not active.