Skip to content
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
34 changes: 21 additions & 13 deletions apps/loopover-extension/content.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,10 @@ function mountOverlay(target) {
} else {
document.body.appendChild(container);
}
const load = createOverlayLoader(container, target);
const refresh = container.querySelector(".loopover-overlay__refresh");
refresh?.addEventListener("click", () => load(container, target));
void load(container, target);
refresh?.addEventListener("click", () => load());
void load();
}

function findPullRequestSidebar() {
Expand All @@ -52,17 +53,23 @@ function findPullRequestSidebar() {
);
}

async function load(container, target) {
const body = container.querySelector(".loopover-overlay__body");
if (!body) return;
body.textContent = "Loading private context...";
const response = await chrome.runtime.sendMessage({ type: "loopover:pull-context", ...target });
if (!response?.ok) {
body.innerHTML = `<div class="loopover-overlay__error">${escapeHtml(response?.error || "Context unavailable")}</div>`;
return;
}
body.innerHTML = renderPullContext(response.payload);
renderActions(body, response.payload?.actions);
function createOverlayLoader(container, target) {
let latestTicket = 0;
return async function load() {
const body = container.querySelector(".loopover-overlay__body");
if (!body) return;
const ticket = ++latestTicket;
body.textContent = "Loading private context...";
const response = await chrome.runtime.sendMessage({ type: "loopover:pull-context", ...target });
// A newer load() has started while this request was in flight; discard the stale response.
if (ticket !== latestTicket) return;
if (!response?.ok) {
body.innerHTML = `<div class="loopover-overlay__error">${escapeHtml(response?.error || "Context unavailable")}</div>`;
return;
}
body.innerHTML = renderPullContext(response.payload);
renderActions(body, response.payload?.actions);
};
}

function renderPullContext(payload) {
Expand Down Expand Up @@ -185,6 +192,7 @@ if (globalThis.__LOOPOVER_EXTENSION_TEST__) {
globalThis.__loopoverContentInternals = {
matchGitHubPageTarget,
matchPullRequestTarget,
createOverlayLoader,
renderPullContext,
renderSection,
renderLegacyPanels,
Expand Down
41 changes: 40 additions & 1 deletion test/unit/extension-content.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,46 @@ describe("extension content script", () => {
expect(html).toContain("public");
expect(html).toContain("no");
});

it("discards an out-of-order refresh response so the overlay keeps the newest request's payload", async () => {
const resolvers: Array<(response: unknown) => void> = [];
const sendMessage = vi.fn(() => new Promise((resolve) => resolvers.push(resolve)));
const internals = loadContentInternals({
chrome: { runtime: { sendMessage } },
});

const body = { innerHTML: "", textContent: "" };
const container = { querySelector: vi.fn(() => body) };
const load = internals.createOverlayLoader(container, {
kind: "pull_request",
owner: "JSONbored",
repo: "loopover",
pullNumber: 42,
});

// Two rapid refresh clicks: the second (newer) request is issued while the first is still in flight.
const first = load();
const second = load();
expect(sendMessage).toHaveBeenCalledTimes(2);
const [resolveFirst, resolveSecond] = resolvers;
if (!resolveFirst || !resolveSecond) throw new Error("expected two in-flight pull-context requests");

// The newer request resolves and renders first...
resolveSecond({ ok: true, payload: { sections: [{ label: "Newer context" }] } });
await second;
expect(body.innerHTML).toContain("Newer context");

// ...then the older, first-issued request resolves last — the out-of-order case. Its stale
// payload must be discarded rather than clobbering the fresher render already on screen.
resolveFirst({ ok: true, payload: { sections: [{ label: "Stale context" }] } });
await first;

expect(body.innerHTML).toContain("Newer context");
expect(body.innerHTML).not.toContain("Stale context");
});
});

function loadContentInternals() {
function loadContentInternals(overrides: Record<string, unknown> = {}) {
const context: Record<string, unknown> = {
__LOOPOVER_EXTENSION_TEST__: true,
location: { pathname: "/JSONbored/loopover/issues/146" },
Expand All @@ -84,6 +121,7 @@ function loadContentInternals() {
body: { appendChild: vi.fn() },
},
chrome: { runtime: { sendMessage: vi.fn() } },
...overrides,
};
context.globalThis = context;
const vmContext = createContext(context);
Expand All @@ -93,6 +131,7 @@ function loadContentInternals() {
pathname: string,
) => { kind: "pull_request"; owner: string; repo: string; pullNumber: number } | { kind: "issue"; owner: string; repo: string; issueNumber: number } | null;
matchPullRequestTarget: (pathname: string) => { owner: string; repo: string; pullNumber: number } | null;
createOverlayLoader: (container: { querySelector: (selector: string) => unknown }, target: unknown) => () => Promise<void>;
renderPullContext: (payload: unknown) => string;
};
}
Loading