Frontend: rate-limit one-more-time requests when page is out of focus or hidden#3828
Open
Williangalvani wants to merge 1 commit intobluerobotics:masterfrom
Open
Frontend: rate-limit one-more-time requests when page is out of focus or hidden#3828Williangalvani wants to merge 1 commit intobluerobotics:masterfrom
Williangalvani wants to merge 1 commit intobluerobotics:masterfrom
Conversation
Reviewer's GuideImplements page focus/visibility–aware throttling for recurring OneMoreTime actions and tracks global page state in the frontend store, including opt-out controls and lifecycle cleanup for listeners. Sequence diagram for OneMoreTime resuming on page focussequenceDiagram
participant Browser as Browser
participant Document as document_window
participant Store as FrontendStore
participant OMT1 as OneMoreTime_1
participant OMT2 as OneMoreTime_2
Note over Document,Store: Initial setup registers visibility and focus listeners
Browser->>Document: User focuses window or tab
Document->>Store: update() calls setPageState(focused)
Store-->>Store: page_state = focused
Note over OMT1,OMT2: Instances previously registered onPageResume in pageResumeListeners
Document->>Document: notify() on focus/visibilitychange
Document->>OMT1: onPageResume()
OMT1->>OMT1: check isDisposed, isPaused, isRunning, timeoutId
alt OMT1 eligible to resume
OMT1->>OMT1: killTask()
OMT1->>OMT1: start()
end
Document->>OMT2: onPageResume()
OMT2->>OMT2: check isDisposed, isPaused, isRunning, timeoutId
alt OMT2 eligible to resume
OMT2->>OMT2: killTask()
OMT2->>OMT2: start()
end
Class diagram for page-aware OneMoreTime throttling and frontend storeclassDiagram
class OneMoreTimeOptions {
+number delay
+number errorDelay
+boolean immediate
+unknown disposeWith
+boolean disablePageThrottle
}
class OneMoreTime {
-OneMoreTimeOptions options
-OneMoreTimeAction action
-boolean isPaused
-boolean isDisposed
-boolean isRunning
-number timeoutId
-function onPageResume()
+constructor(options OneMoreTimeOptions, action OneMoreTimeAction)
-function getEffectiveDelay(baseDelay number) number
-function watchDisposeWith() void
-function killTask() void
+function pause() void
+function resume() void
+function softStart() void
+function start() Promise~void~
+function [Symbol.dispose]() void
}
class FrontendStore {
+boolean backend_offline
+PageState page_state
+string frontend_id
+function setBackendOffline(offline boolean) void
+function setPageState(state PageState) void
}
class PageState {
<<type>>
focused
blurred
hidden
}
OneMoreTime --> OneMoreTimeOptions : uses
OneMoreTime --> PageState : reads via frontend.page_state
FrontendStore --> PageState : manages
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
cd78d01 to
4f99354
Compare
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The module-level listeners in
one-more-time.tsandstore/frontend.tsare never torn down, which can cause leaks or duplicated handlers in HMR/multi-mount scenarios; consider scoping them to the app lifecycle (e.g., registering on app init and removing on teardown). - In
one-more-time.ts,window.addEventListeneris called inside atypeof document !== 'undefined'guard but without checkingtypeof window !== 'undefined', which can break in non-browser/SSR-like environments wheredocumentandwindowlifecycles differ; it would be safer to guard both globals.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The module-level listeners in `one-more-time.ts` and `store/frontend.ts` are never torn down, which can cause leaks or duplicated handlers in HMR/multi-mount scenarios; consider scoping them to the app lifecycle (e.g., registering on app init and removing on teardown).
- In `one-more-time.ts`, `window.addEventListener` is called inside a `typeof document !== 'undefined'` guard but without checking `typeof window !== 'undefined'`, which can break in non-browser/SSR-like environments where `document` and `window` lifecycles differ; it would be safer to guard both globals.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Adjust OneMoreTime polling behavior to respect page visibility and focus, reducing background activity while keeping data fresh when the page is active.
Enhancements: