fix: stop the file-viewer reload loop on watched files
Nightly Build / build (push) Successful in 7m35s
Nightly Build / build (push) Successful in 7m35s
The watch callback forwarded every FS event, including the pure reads the viewer's own GET /api/file produces (IN_ACCESS / IN_CLOSE_NOWRITE on Linux): each silent reload re-triggered the watcher, looping at ~1 Hz. For PDFs every iteration minted a new blob URL and re-assigned iframe.src, which re-runs Chrome's whole PDF viewer (the flicker) and pushes a joint session-history entry (the back button buried under hundreds of blob: entries). - file_watch: forward an event only when the content version (mtime_ns, len) actually moved; drop Access events outright, stat-compare the rest. - viewer: render pdf/latex/svg previews in a keyed() iframe — a fresh element's first navigation replaces its history slot instead of pushing.
This commit is contained in:
@@ -29,6 +29,13 @@
|
|||||||
//! sharing). On disconnect every watcher is dropped and the OS resources are
|
//! sharing). On disconnect every watcher is dropped and the OS resources are
|
||||||
//! released automatically.
|
//! released automatically.
|
||||||
//!
|
//!
|
||||||
|
//! ## What counts as a change
|
||||||
|
//!
|
||||||
|
//! Only events that move the file's content version (mtime + len) are
|
||||||
|
//! forwarded; pure reads (`Access`) and metadata-only touches are dropped in
|
||||||
|
//! the watcher callback. Without that filter the viewer's own `GET /api/file`
|
||||||
|
//! read would re-trigger the watcher on every reload, looping forever.
|
||||||
|
//!
|
||||||
//! ## LaTeX dependency-aware watching
|
//! ## LaTeX dependency-aware watching
|
||||||
//!
|
//!
|
||||||
//! When subscribing to a `.tex` / `.latex` source, the server expands the
|
//! When subscribing to a `.tex` / `.latex` source, the server expands the
|
||||||
@@ -59,7 +66,7 @@ use axum::{
|
|||||||
response::IntoResponse,
|
response::IntoResponse,
|
||||||
};
|
};
|
||||||
use core_api::user_fs::SharedFs;
|
use core_api::user_fs::SharedFs;
|
||||||
use notify::{Config, RecommendedWatcher, RecursiveMode, Watcher};
|
use notify::{Config, EventKind, RecommendedWatcher, RecursiveMode, Watcher};
|
||||||
use serde::Deserialize;
|
use serde::Deserialize;
|
||||||
use serde_json::json;
|
use serde_json::json;
|
||||||
use tokio::sync::mpsc;
|
use tokio::sync::mpsc;
|
||||||
@@ -216,17 +223,35 @@ fn install_watcher(
|
|||||||
for path in paths_to_watch {
|
for path in paths_to_watch {
|
||||||
let tx_for_cb = change_tx.clone();
|
let tx_for_cb = change_tx.clone();
|
||||||
let original_path = user_path.to_string();
|
let original_path = user_path.to_string();
|
||||||
|
let path_for_cb = path.clone();
|
||||||
|
// Content-version baseline for the stat dedup in the callback.
|
||||||
|
let mut last_stamp = content_stamp(&path);
|
||||||
|
|
||||||
let mut watcher = RecommendedWatcher::new(
|
let mut watcher = RecommendedWatcher::new(
|
||||||
move |res: notify::Result<notify::Event>| {
|
move |res: notify::Result<notify::Event>| {
|
||||||
// Any event on the watched path triggers a change notification.
|
let Ok(event) = res else { return };
|
||||||
// We don't inspect the event kind — reload on the client side
|
// A pure read is never a change — and it matters doubly here,
|
||||||
// re-reads the file and naturally handles create/modify/remove.
|
// because the viewer's own `GET /api/file` produces exactly
|
||||||
if res.is_ok() {
|
// these events (IN_ACCESS / IN_CLOSE_NOWRITE on Linux):
|
||||||
|
// forwarding them made every client reload re-trigger the
|
||||||
|
// watcher, a self-sustaining reload loop. Real writes still
|
||||||
|
// surface as Modify events, so dropping Access loses nothing.
|
||||||
|
if matches!(event.kind, EventKind::Access(_)) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
// Every other kind is answered with the one question that
|
||||||
|
// matters — did (mtime, len) actually move? This swallows
|
||||||
|
// metadata-only noise (atime, chmod) whatever kind the backend
|
||||||
|
// reports it as, without trusting per-platform kind mappings.
|
||||||
|
// A failed stat means the file is gone, and that IS a change.
|
||||||
|
let stamp = content_stamp(&path_for_cb);
|
||||||
|
if stamp == last_stamp {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
last_stamp = stamp;
|
||||||
if tx_for_cb.send(original_path.clone()).is_err() {
|
if tx_for_cb.send(original_path.clone()).is_err() {
|
||||||
// channel closed — receiver dropped (WS disconnected).
|
// channel closed — receiver dropped (WS disconnected).
|
||||||
}
|
}
|
||||||
}
|
|
||||||
},
|
},
|
||||||
Config::default(),
|
Config::default(),
|
||||||
)
|
)
|
||||||
@@ -248,6 +273,20 @@ fn install_watcher(
|
|||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Content version of a file as (mtime_ns, len) — the same inputs as the HTTP
|
||||||
|
/// `ETag` (`disk_etag` in `api/files.rs`). A pure read never moves it, any
|
||||||
|
/// write does. `None` when the file is missing or unreadable.
|
||||||
|
fn content_stamp(path: &Path) -> Option<(u128, u64)> {
|
||||||
|
let md = std::fs::metadata(path).ok()?;
|
||||||
|
let mtime_ns = md
|
||||||
|
.modified()
|
||||||
|
.ok()?
|
||||||
|
.duration_since(std::time::UNIX_EPOCH)
|
||||||
|
.ok()?
|
||||||
|
.as_nanos();
|
||||||
|
Some((mtime_ns, md.len()))
|
||||||
|
}
|
||||||
|
|
||||||
/// True for `.tex` / `.latex` extensions — sources that trigger the
|
/// True for `.tex` / `.latex` extensions — sources that trigger the
|
||||||
/// dependency-aware watcher expansion.
|
/// dependency-aware watcher expansion.
|
||||||
fn is_latex_path(path: &str) -> bool {
|
fn is_latex_path(path: &str) -> bool {
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
import { html, nothing } from 'lit';
|
import { html, nothing } from 'lit';
|
||||||
import { unsafeHTML } from 'lit/directives/unsafe-html.js';
|
import { unsafeHTML } from 'lit/directives/unsafe-html.js';
|
||||||
|
import { keyed } from 'lit/directives/keyed.js';
|
||||||
import { LightElement, renderMarkdown } from '../../lib/base.js';
|
import { LightElement, renderMarkdown } from '../../lib/base.js';
|
||||||
import { fileWatcher } from '../../lib/file-watcher.js';
|
import { fileWatcher } from '../../lib/file-watcher.js';
|
||||||
import { t } from '../../lib/i18n.js';
|
import { t } from '../../lib/i18n.js';
|
||||||
@@ -553,12 +554,17 @@ export class FileViewerBase extends LightElement {
|
|||||||
return html`<div class="fv-image-wrap"><img src=${this._blobUrl} alt=${this._path} class="fv-image" /></div>`;
|
return html`<div class="fv-image-wrap"><img src=${this._blobUrl} alt=${this._path} class="fv-image" /></div>`;
|
||||||
}
|
}
|
||||||
if (this._kind === 'pdf' && this._blobUrl) {
|
if (this._kind === 'pdf' && this._blobUrl) {
|
||||||
return html`<iframe class="fv-pdf" src=${this._blobUrl} title=${this._path}></iframe>`;
|
// `keyed` re-creates the iframe element on every new blob URL: the first
|
||||||
|
// navigation of a fresh iframe replaces its history slot instead of
|
||||||
|
// pushing one — whereas re-assigning `src` on an existing iframe pushes
|
||||||
|
// a joint session-history entry each time (during the watch-reload loop
|
||||||
|
// that buried the back button under hundreds of blob: entries).
|
||||||
|
return keyed(this._blobUrl, html`<iframe class="fv-pdf" src=${this._blobUrl} title=${this._path}></iframe>`);
|
||||||
}
|
}
|
||||||
if (this._kind === 'latex' && this._blobUrl) {
|
if (this._kind === 'latex' && this._blobUrl) {
|
||||||
// Successfully compiled server-side — render the resulting PDF the same
|
// Successfully compiled server-side — render the resulting PDF the same
|
||||||
// way a native .pdf would be rendered.
|
// way a native .pdf would be rendered (see the keyed() note above).
|
||||||
return html`<iframe class="fv-pdf" src=${this._blobUrl} title=${this._path}></iframe>`;
|
return keyed(this._blobUrl, html`<iframe class="fv-pdf" src=${this._blobUrl} title=${this._path}></iframe>`);
|
||||||
}
|
}
|
||||||
if (this._kind === 'svg' && this._blobUrl) {
|
if (this._kind === 'svg' && this._blobUrl) {
|
||||||
// `allow-same-origin` (and nothing else) is required so the iframe can load
|
// `allow-same-origin` (and nothing else) is required so the iframe can load
|
||||||
@@ -566,7 +572,7 @@ export class FileViewerBase extends LightElement {
|
|||||||
// `allow-scripts` absent, any <script> inside the SVG still cannot execute,
|
// `allow-scripts` absent, any <script> inside the SVG still cannot execute,
|
||||||
// so this stays an isolated, script-free render.
|
// so this stays an isolated, script-free render.
|
||||||
return html`<div class="fv-image-wrap">
|
return html`<div class="fv-image-wrap">
|
||||||
<iframe class="fv-svg" sandbox="allow-same-origin" src=${this._blobUrl} title=${this._path}></iframe>
|
${keyed(this._blobUrl, html`<iframe class="fv-svg" sandbox="allow-same-origin" src=${this._blobUrl} title=${this._path}></iframe>`)}
|
||||||
</div>`;
|
</div>`;
|
||||||
}
|
}
|
||||||
if (this._kind === 'binary') {
|
if (this._kind === 'binary') {
|
||||||
|
|||||||
@@ -78,6 +78,7 @@
|
|||||||
"imports": {
|
"imports": {
|
||||||
"lit": "/vendor/lit-all.min.js",
|
"lit": "/vendor/lit-all.min.js",
|
||||||
"lit/directives/unsafe-html.js": "/vendor/lit-all.min.js",
|
"lit/directives/unsafe-html.js": "/vendor/lit-all.min.js",
|
||||||
|
"lit/directives/keyed.js": "/vendor/lit-all.min.js",
|
||||||
"marked": "/vendor/marked.esm.js",
|
"marked": "/vendor/marked.esm.js",
|
||||||
"dompurify": "/vendor/purify.es.mjs",
|
"dompurify": "/vendor/purify.es.mjs",
|
||||||
"cronstrue": "/vendor/cronstrue.mjs"
|
"cronstrue": "/vendor/cronstrue.mjs"
|
||||||
|
|||||||
@@ -52,6 +52,7 @@
|
|||||||
"imports": {
|
"imports": {
|
||||||
"lit": "/vendor/lit-all.min.js",
|
"lit": "/vendor/lit-all.min.js",
|
||||||
"lit/directives/unsafe-html.js": "/vendor/lit-all.min.js",
|
"lit/directives/unsafe-html.js": "/vendor/lit-all.min.js",
|
||||||
|
"lit/directives/keyed.js": "/vendor/lit-all.min.js",
|
||||||
"marked": "/vendor/marked.esm.js",
|
"marked": "/vendor/marked.esm.js",
|
||||||
"dompurify": "/vendor/purify.es.mjs"
|
"dompurify": "/vendor/purify.es.mjs"
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user