Skip to content

Seamless asset refresh instead of the reload prompt - #4810

Closed
ernestdefoe wants to merge 1 commit into
flarum:2.xfrom
ernestdefoe:seamless-asset-refresh
Closed

Seamless asset refresh instead of the reload prompt#4810
ernestdefoe wants to merge 1 commit into
flarum:2.xfrom
ernestdefoe:seamless-asset-refresh

Conversation

@ernestdefoe

Copy link
Copy Markdown

Background

When the forum's assets are rebuilt — a deploy, or simply an admin enabling/disabling an extension — every API response starts carrying a new X-Flarum-Assets-Revision. ForumApplication.checkAssetsRevision compares it to the revision the page booted with and, if it differs, immediately shows a dismissible "A new version of this page is available — Reload" alert.

Two problems with showing it right away:

  • It appears mid-read or mid-compose, interrupting whatever the user was doing.
  • Because an extension toggle rebuilds assets, it pops up for every visitor on the forum at once the moment an admin touches the admin area — which reads as the forum nagging everyone to reload.

What this does

Implements the middle ground @luceos proposed — don't interrupt, don't ignore. When newer assets are detected we just remember it (assetsRefreshPending) and say nothing. The user's next real navigation — clicking a link, opening a discussion, back/forward — is turned into a full page load, so the fresh assets are picked up naturally.

The result: an actively reading or typing user is never interrupted, there's no modal and no timing guesswork, no risk of discarding an open draft — and the very next navigation is already on the new assets.

Details

  • checkAssetsRevision now sets a flag and lazily wires up refreshOnNextNavigation() instead of showing the alert. It's still public so the realtime extension's pushed revision token flows through the same path.
  • refreshOnNextNavigation() adds a capture-phase click listener (stops propagation so it runs before the router's <Link> handling) that only acts on a plain left-click of an internal, same-origin link. It deliberately leaves untouched: modified clicks (Ctrl/⌘/Shift/middle → open in new tab/window), target="_blank" / download links, in-page anchors (#…), javascript: / mailto: / tel: links, external links, and bare hash changes on the current page. popstate reloads too.
  • Removed the now-unused Button import.

The core.lib.assets_updated.message / .reload_button locale strings are now unused. I left them in place to avoid churning existing translations — happy to remove them if you'd prefer.

Testing

Verified against a forum whose assets were rebuilt after load:

  • No alert appears; the flag is set instead.
  • The next internal-link click / back-forward triggers a full page load onto the new assets, while modified clicks, new-tab/download, external, #, javascript: links, and same-page hash changes all behave normally.

I've also been running this as a userland extension (overriding the same method) on two production forums.

When the forum's assets are rebuilt (a deploy, or an admin toggling an
extension), core detects the new revision and immediately shows a dismissible
"a new version of this page is available — reload" alert. It appears mid-read
or mid-compose, and because an extension toggle rebuilds assets, it pops up for
every visitor on the forum at once.

Following luceos's proposal, don't interrupt: when newer assets are detected,
flag it and turn the user's next navigation into a full page load, so fresh
assets are picked up naturally without ever interrupting a reading or typing
session. Modified clicks, new-tab/download links, in-page anchors and external
links are left untouched; back/forward reloads too.
@ernestdefoe
ernestdefoe requested a review from a team as a code owner July 8, 2026 17:33
@imorland

Copy link
Copy Markdown
Member

Thanks for the writeup — and the observation underneath it is correct: the reload prompt was appearing far too often, for rebuilds that changed nothing the visitor had loaded. That part is a real bug, and it deserved fixing.

But this PR doesn't fix it — it hides it. The prompt was firing spuriously: the asset revision was derived from source file mtimes, and an extension toggle flushed the manifest to be lazily rebuilt, so the token moved on every toggle and redeploy even when the compiled output was byte-identical. Swapping the alert for a silent reload-on-next-navigation doesn't change any of that — it converts a spurious alert into a spurious forced full page load on the next click, for every visitor, still for changes that don't affect them. The signal is still wrong; it's just quieter. The same applies to the extension you've published implementing this approach: it carries the same flaw, for the same reason — the symptom is suppressed while the detection underneath keeps misfiring.

The premise also doesn't hold up. The alert doesn't interrupt anything: it's a dismissible, non-blocking notice, and a user who's mid-composition simply carries on composing with it in view. That is the entire point — they are aware a reload is needed and choose when to act on it. This PR replaces that with a hard reload embedded into a click the user made for a different reason. A full page load the user didn't ask for, at a moment they don't expect, is far more likely to read as "the forum is broken" than the alert ever was — and that means confusion, bug reports, and support burden.

More broadly: papering over cracks is not something we accept into Flarum. It's bad practice and it isn't sustainable — the crack is still there, now harder to see, and the covering itself becomes surface we maintain forever. Here that covering is a document-level capture-phase click interceptor that stops propagation and re-implements link semantics (modified clicks, target="_blank", download, hash anchors, external URLs, javascript:/mailto:/tel:, popstate) — one missed case and navigation breaks in ways users can barely describe. When proposing a fix or a feature, deep consideration needs to go into whether the change is even at the right layer. This one treats presentation; the defect was in detection — and the untouched detection layer also missed genuine changes written within the same second, and left a window where already-served asset URLs 404'd after a flush.

The root cause is addressed in #4857: revisions are now derived from the compiled output (so the token changes if and only if the bytes a client would download change), and toggles mark assets for an in-place rebuild on the next request instead of flushing them. With that in place, the prompt only appears when the assets a client is running have genuinely been superseded — which is exactly when a visible, user-controlled prompt is the right UX. Worth noting: once that lands, the situation your extension was built around largely disappears — prompts become rare and genuine — so it's worth re-evaluating whether the navigation-hijack trade-off still earns its keep.

So I'm going to close this one. The problem you identified was real and I'm glad it was raised — but the fix needed to happen in detection, not presentation.

@imorland imorland closed this Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants