feat: per-widget refresh-interval and force-refresh endpoint#1005
Open
jrnxf wants to merge 7 commits intoglanceapp:mainfrom
Open
feat: per-widget refresh-interval and force-refresh endpoint#1005jrnxf wants to merge 7 commits intoglanceapp:mainfrom
jrnxf wants to merge 7 commits intoglanceapp:mainfrom
Conversation
Replaces the page-level lock around updateOutdatedWidgets with a
per-widget mutex on widgetBase, taken inside renderTemplate (covering
all .Render paths) and externally around update() calls.
Implements the previously-stubbed handleWidgetRequest as a refresh
endpoint at GET /api/widgets/{widget}/refresh. ?force=1 bypasses the
widget's cache; otherwise the existing cache duration applies. Returns
the rendered widget HTML for outerHTML replacement on the client.
Adds a refresh-interval YAML field on widgetBase and validates it at
config load. Disallowed on widget types that hold interactive client
state, embed external content, or have nothing to refetch (calendar,
to-do, clock, iframe, html, search, bookmarks, group, split-column).
Minimum interval is 30s.
The widget root in widget-base.html now exposes data-widget-id and
data-refresh-interval so the frontend scheduler (separate commit) can
discover refreshable widgets without server coordination.
Adds widget-refresh.js, a scheduler that polls each widget's
/api/widgets/{id}/refresh endpoint at its configured interval and
swaps the widget's outerHTML. Pauses while document.hidden and
fires an immediate refresh on visibility resume if the interval
has elapsed.
The page wasn't built with a widget lifecycle, so rather than
adding one this introduces a small per-widget cleanup-callback
registry stored on the widget's root element. Setups that bind to
window/document or create observers (carousel resize, masonry and
collapsible-grid ResizeObservers) register a cleanup callback;
runWidgetCleanup runs them before the outerHTML swap so listeners
and observers don't accumulate as widgets refresh.
Setup functions in page.js, popover.js, and masonry.js now accept
an optional root argument so they can be re-run scoped to a single
widget after refresh, via the new reinitWidget helper. Popovers and
lazy-image listeners are bound to widget descendants and don't need
cleanup tracking - they're released when the old DOM is replaced.
setupDynamicRelativeTime now queries the document fresh on each
tick and is initialized only once, so relative-time elements
introduced by widget refreshes get picked up automatically.
afterContentReady fires immediately when called after the initial
content-ready signal, so post-refresh setups don't queue forever.
Docs: shared-properties table and a refresh-interval section in
configuration.md, including the disallowed-types note and the
visibility-pause behavior.
Upstream rate limiting is the job of the widget's cache duration, not the client refresh interval. A refresh-interval tick that lands inside the cache window simply re-renders existing data — no upstream call. The 30s floor was over-cautious; 5s is enough to prevent a 1s-typo footgun while letting users dial in a tight local-feel cadence on widgets like monitor or server-stats whose cache controls the actual fetch rate.
fb597cb to
b64e37e
Compare
…erval on 404 Three issues caught in review: - Widgets nested inside group/split-column weren't added to widgetByID, so their refresh-interval ticks would 404 forever. Walk container children recursively when registering. - updateOutdatedWidgets and containerWidgetBase._update checked requiresUpdate outside the per-widget lock, so two concurrent page renders could both observe an outdated widget and both fire upstream. Re-check inside the lock. - The client-side scheduler kept retrying on 404 forever (e.g. after a config reload reassigns ids). Drop the interval and let a full page reload repopulate. Also adds the missing per-widget lock to the Columns loop in updateOutdatedWidgets — only the HeadWidgets loop had it before.
Element.closest is universally supported and returns Element|null — the && guard and the undefined check were dead code.
Tests prove the in-lock requiresUpdate re-check actually dedupes: fire 10 concurrent updateOutdatedWidgets / _update calls against a counter widget, expect exactly one update. Also trims three comments that were heavier than the codebase's typical style — the registerWidget doc, the minRefreshInterval rationale, and the RefreshIntervalSeconds doc-comment (function name is self-explanatory).
Author
|
@svilenmarkov any chance I could get your eyes on this? Think it would be a big add a lot of people would benefit from! 🍻 |
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.
Closes #327 (request).
This is a fresh take on per-widget refresh, written specifically around the two
concerns you raised on that issue and on #793:
All are addressed below.
What it does
refresh-interval: <duration>field on every widget. When set, theclient polls
GET /api/widgets/{id}/refreshon that interval and swaps thewidget's
outerHTMLin place.GET /api/widgets/{widget}/refresh(the route was alreadyreserved with a TODO comment in
glance.go).?force=1bypasses thewidget's cache; without it, a tick within the cache window just re-renders
existing data (no upstream call). This is what the existing
cache:fieldbecomes useful for once you have client polling.
validate()call at config load rejectsrefresh-intervalon widgettypes that hold interactive client state, embed external content, or have
nothing to refetch (
calendar,to-do,clock,iframe,html,search,bookmarks,group,split-column). Fails fast at config loadrather than silently ignoring. Minimum 5s as a typo-floor.
Addressing your concerns
"the frontend wasn't built to unmount" — handled, no global lifecycle refactor
I deliberately did not introduce a generic widget lifecycle. Instead:
setupX()functions inpage.js,popover.js,masonry.jsnowaccept an optional
root = documentargument. Existing call sites passnothing and behave identically.
_cleanupCallbackson thewidget's root element) — only the three setups that bind to
window/documentor instantiate observers register a callback:setupCarousels(window resize listener),setupCollapsibleGridsandsetupMasonries(ResizeObserver). All other setups bind to widgetdescendants and are released naturally when the old DOM is replaced.
runWidgetCleanup(widgetEl)runs those callbacks beforeouterHTMLisswapped. The new node has its own fresh registry.
setupDynamicRelativeTimewas the one global timer that captured a staleelementsarray — it now queries the document fresh on each tick and isinitialized only once. Refreshed widgets are picked up automatically.
afterContentReadyfires immediately when called post-init, so refresh-path setups don't queue forever.
I verified there's no listener accumulation by leaving a horizontal-cards RSS
widget refreshing for several cycles in a real browser and watching its
_cleanupCallbackscount — it stays at 1, not climbing."many times more upstream requests" — the math doesn't actually pencil out that way
The whole mechanism is built around
cache:being the rate limiter, not theclient interval:
requiresUpdate→ false →re-renders the existing buffer. Zero upstream calls.
update(),serialized by the per-widget mutex (and re-checked inside the lock so two
concurrent renders can't double-fire).
So with the typical RSS default of
cache: 1handrefresh-interval: 1m,that's 60 client ticks per hour producing 1 upstream call per hour — the
same call cadence you have today, just delivered to the browser without a
manual reload. The user has to set
cache:to something tighter to actuallyincrease upstream traffic, which is the same lever that exists today.
"only update while in foreground" — done
visibilitychangeclears all interval handles when the tab is hidden andreinstalls them on focus, with an immediate refresh if the interval already
elapsed during the hidden period. Tab in the background → zero requests.
"should be per-widget basis" — done
Opt-in per widget, default off. Disallow list enforced at config load.
"how is this different from a browser refresh?" — three things
requiresUpdateshort-circuits when the cache is fresh. You can't actuallypull new data on demand without restarting the server.
?force=1does.other widgets, etc.
server-stats each have their own cadence without a page-wide reload that
also refetches feeds and other slow widgets.
Locking
The previously-stubbed
handleWidgetRequesthad a TODO calling out thelocking rework. That's the other half of the backend commit:
widgetBasegets async.Mutex.renderTemplatetakes the lock (covers all.Renderpaths via theshared
templateBuffer).updateOutdatedWidgetsandcontainerWidgetBase._updatelock per-widgetaround
update(ctx)and re-checkrequiresUpdateinside the lock so twoconcurrent page renders can't both fire the same upstream call.
updateOutdatedWidgetsis removed (and theunused
page.mufield with it).Render()andupdate()don't recursively call each other in any widgettype, so re-entrance isn't a risk. Containers (group, split-column) don't
have their own mutex contention with children — different mutex objects.
What's not in this PR
stays focused on the mechanism.
Test notes
TestWidgetValidateRefreshIntervalcovers the disallow list, theminimum-interval floor, and the no-interval baseline.
TestRegisterWidgetIncludesContainerChildrencovers recursiveregistration of nested widgets.
TestUpdateOutdatedWidgetsDedupesConcurrentCallsandTestContainerUpdateDedupesConcurrentCallsfire 10 concurrent rendersagainst a counter-stub widget and assert exactly one
update()fires —evidence that the locking scheme actually delivers the API politeness
claim above.
hacker-news,rss(list +horizontal-cards/carousel),
monitor,server-stats,dns-stats,custom-api. Auto-refresh swaps cleanly, popovers and lazy-images keepworking post-swap, console stays clean across cycles.
immediate refresh if interval elapsed.
Happy to split the PR, change naming, restructure the cleanup registry, or
rewrite the description — just let me know what fits the project's direction.