Skip to content
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

fix: stale live reload due to dropped watch events #649

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ Go to the `v1` branch to see the changelog of Lume 1.
- Watcher new files on Windows.
- Feed plugin: error when the updated/published value is a string [#638].
- Fixed esbuild reload [#647].
- Fixed serve showing stale pages [#649].
- Speed up logging to console with colors [#651]

## [2.2.4] - 2024-07-18
Expand Down
48 changes: 31 additions & 17 deletions core/watcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,30 +77,31 @@ export default class FSWatcher implements Watcher {
async start() {
const { root, ignore, debounce } = this.options;
const watcher = Deno.watchFs(root);
const changes = new Set<string>();
const changeQueue: Set<string>[] = [];
let timer = 0;
let runningCallback = false;

await this.dispatchEvent({ type: "start" });

const callback = async () => {
if (!changes.size || runningCallback) {
return;
}

const files = new Set(changes);
changes.clear();

runningCallback = true;

try {
const result = await this.dispatchEvent({ type: "change", files });
if (false === result) {
return watcher.close();
let changes: Set<string> | undefined;
while ((changes = changeQueue.pop()) !== undefined) {
try {
const result = await this.dispatchEvent({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of dispatching an event per change in the queue, why don't merge all changes and dispatch the event only once? I mean:

let changes: Set<string> | undefined;
let mergedChanges = new Set<string>();

while ((changes = changeQueue.pop()) !== undefined) {
  mergedChanges = mergedChanges.union(changes);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, it didn't even occur to me that we could batch all future changes into the same one. This is a great idea!

type: "change",
files: changes,
});
if (false === result) {
runningCallback = false;
return watcher.close();
}
} catch (error) {
await this.dispatchEvent({ type: "error", error });
}
} catch (error) {
await this.dispatchEvent({ type: "error", error });
}

runningCallback = false;
};

Expand All @@ -123,11 +124,24 @@ export default class FSWatcher implements Watcher {
continue;
}

const changes = new Set<string>();
paths.forEach((path) => changes.add(normalizePath(relative(root, path))));

// Debounce
clearTimeout(timer);
timer = setTimeout(callback, debounce ?? 100);
// If we're already processing and have a pending
// queue item, we can merge all future changes together
if (runningCallback && changeQueue.length > 0) {
const last = changeQueue[changeQueue.length - 1];
changeQueue[changeQueue.length - 1] = last.union(changes);
} else {
changeQueue.unshift(changes);

// Only start if processing queue is not already running
if (!runningCallback) {
// Debounce
clearTimeout(timer);
timer = setTimeout(callback, debounce ?? 100);
}
}
}
}
}
Expand Down
40 changes: 36 additions & 4 deletions middlewares/reload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,33 @@ export function reload(options: Options): Middleware {
const sockets = new Set<WebSocket>();
const { watcher } = options;

// Keep track of the change revision. A watch change
// can be dispatched in-between the browser loading
// the HTML and before it has established a WebSocket
// connection. In this case the browser is out of sync
// and shows an old version of the page. Upon establishing
// a websocket connection we send the latest revision
// and the browser can potentially refresh itself when
// it has an older revision. The initial revision is
// sent to the browser as part of the HTML.
let revision = 0;
let lastAcknowledgedRevision = 0;

watcher.addEventListener("change", (event) => {
revision++;

if (!sockets.size) {
return;
}

lastAcknowledgedRevision = revision;

const files = event.files!;
const urls = Array.from(files).map((file) => normalizePath(file));
const message = JSON.stringify(urls);
const message = JSON.stringify({
type: "update",
revision,
files: Array.from(files).map((file) => normalizePath(file)),
});
sockets.forEach((socket) => {
if (socket.readyState === WebSocket.OPEN) {
socket.send(message);
Expand All @@ -36,7 +55,19 @@ export function reload(options: Options): Middleware {
if (request.headers.get("upgrade") === "websocket") {
const { socket, response } = Deno.upgradeWebSocket(request);

socket.onopen = () => sockets.add(socket);
socket.onopen = () => {
// Browser was in the process of being reloaded. Notify
// the user that the latest changes were sent.
if (lastAcknowledgedRevision < revision) {
lastAcknowledgedRevision = revision;
console.log("Changes sent to the browser");
}

// Tell the browser about the most recent revision
socket.send(JSON.stringify({ type: "init", revision }));

sockets.add(socket);
};
socket.onclose = () => sockets.delete(socket);
socket.onerror = (e) => console.log("Socket errored", e);

Expand All @@ -63,8 +94,9 @@ export function reload(options: Options): Middleware {
result = await reader.read();
}

// Add live reload script and pass initial revision
body +=
`<script type="module" id="lume-live-reload">${reloadClient}; liveReload();</script>`;
`<script type="module" id="lume-live-reload">${reloadClient}; liveReload(${revision});</script>`;

const { status, statusText, headers } = response;

Expand Down
23 changes: 18 additions & 5 deletions middlewares/reload_client.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
export default function liveReload() {
export default function liveReload(initRevision) {
let ws;
let wasClosed = false;
let revision = initRevision;

function socket() {
if (ws && ws.readyState !== 3) {
Expand All @@ -26,14 +27,26 @@ export default function liveReload() {
}
};
ws.onmessage = (e) => {
const files = JSON.parse(e.data);
const message = JSON.parse(e.data);

if (!Array.isArray(files)) {
console.log(e.data);
if (message.type === "init" && message.revision > revision) {
location.reload();
return;
}

refresh(files);
// Always update revision
revision = message.revision;

if (message.type === "update") {
const files = message.files;

if (!Array.isArray(message.files)) {
console.log(e.data);
return;
}

refresh(files);
}
};
ws.onclose = () => {
wasClosed = true;
Expand Down
Loading