From b8180c52ec7a37484df65090dad49d01b758b2b1 Mon Sep 17 00:00:00 2001 From: shawn Date: Thu, 30 Jul 2026 11:04:22 +0800 Subject: [PATCH 1/3] fix: load pr branches for filtered lists --- src/features/pr-branch-names.test.ts | 14 ++++++ src/features/pr-branch-names.ts | 9 +++- src/lib/github-api.test.ts | 10 ++--- src/lib/github-api.ts | 2 + src/lib/messages.ts | 2 +- src/service-worker.test.ts | 66 ++++++++++++++++++++++++++-- src/service-worker.ts | 48 ++++++++++++++++++-- 7 files changed, 136 insertions(+), 15 deletions(-) diff --git a/src/features/pr-branch-names.test.ts b/src/features/pr-branch-names.test.ts index 479a9cb..8893042 100644 --- a/src/features/pr-branch-names.test.ts +++ b/src/features/pr-branch-names.test.ts @@ -41,6 +41,20 @@ describe("injectPRBranchNames", () => { expect(document.querySelectorAll(".bg-skeleton-pill--branch")).toHaveLength(0); }); + it("requests the visible PR numbers on filtered search pages", async () => { + setUrl(`${GH}/owner/repo/pulls?q=pre+bid+sort%3Aupdated-desc+`); + twoPRRows(); + vi.mocked(fetchPRBranches).mockResolvedValue([ + { number: 7, headRef: "feature/a" }, + { number: 8, headRef: "fix/b" }, + ]); + + await injectPRBranchNames(); + + expect(fetchPRBranches).toHaveBeenCalledWith("owner", "repo", [7, 8], "open", 1); + expect(document.querySelectorAll(".better-github-branch-badge")).toHaveLength(2); + }); + it("ignores PR numbers the API did not return", async () => { twoPRRows(); vi.mocked(fetchPRBranches).mockResolvedValue([{ number: 7, headRef: "feature/a" }]); diff --git a/src/features/pr-branch-names.ts b/src/features/pr-branch-names.ts index f1fcc86..f4f759a 100644 --- a/src/features/pr-branch-names.ts +++ b/src/features/pr-branch-names.ts @@ -50,14 +50,19 @@ export async function injectPRBranchNames(): Promise { const existing = document.querySelectorAll(`.${BADGE_CLASS}`); if (existing.length > 0) return; + const prRows = document.querySelectorAll("[id^='issue_']:not([id$='_link'])"); + const prNumbers = [...prRows] + .map((row) => Number(row.id.replace("issue_", ""))) + .filter(Number.isInteger); + if (prNumbers.length === 0) return; + try { const { state, page } = getPRListParams(); - const branches = await fetchPRBranches(info.owner, info.repo, state, page); + const branches = await fetchPRBranches(info.owner, info.repo, prNumbers, state, page); if (branches.length === 0) return; const branchMap = new Map(branches.map((b) => [b.number, b.headRef])); - const prRows = document.querySelectorAll("[id^='issue_']"); for (const row of prRows) { const id = row.getAttribute("id"); diff --git a/src/lib/github-api.test.ts b/src/lib/github-api.test.ts index 2d0c53c..ee59554 100644 --- a/src/lib/github-api.test.ts +++ b/src/lib/github-api.test.ts @@ -45,28 +45,28 @@ describe("github-api bridge", () => { it("forwards a typed request and resolves the worker's data on success", async () => { const runtime = mockRuntime({ response: { ok: true, data: [{ number: 7, headRef: "feature/a" }] } }); - const result = await fetchPRBranches("owner", "repo", "open", 2); + const result = await fetchPRBranches("owner", "repo", [7], "open", 2); expect(result).toEqual([{ number: 7, headRef: "feature/a" }]); expect(runtime.sendMessage).toHaveBeenCalledWith( - { type: "FETCH_PR_BRANCHES", owner: "owner", repo: "repo", state: "open", page: 2 }, + { type: "FETCH_PR_BRANCHES", owner: "owner", repo: "repo", prNumbers: [7], state: "open", page: 2 }, expect.any(Function), ); }); it("swallows an ok:false response and returns the empty default", async () => { mockRuntime({ response: { ok: false, error: "boom" } }); - expect(await fetchPRBranches("owner", "repo")).toEqual([]); + expect(await fetchPRBranches("owner", "repo", [1])).toEqual([]); }); it("treats chrome.runtime.lastError as a failure", async () => { mockRuntime({ response: { ok: true, data: [] }, lastError: { message: "port closed" } }); - expect(await fetchPRBranches("owner", "repo")).toEqual([]); + expect(await fetchPRBranches("owner", "repo", [1])).toEqual([]); }); it("rejects without messaging when the extension context is invalidated", async () => { const runtime = mockRuntime({ id: undefined }); - expect(await fetchPRBranches("owner", "repo")).toEqual([]); + expect(await fetchPRBranches("owner", "repo", [1])).toEqual([]); expect(runtime.sendMessage).not.toHaveBeenCalled(); }); diff --git a/src/lib/github-api.ts b/src/lib/github-api.ts index d6b9750..4e88bb9 100644 --- a/src/lib/github-api.ts +++ b/src/lib/github-api.ts @@ -79,6 +79,7 @@ export async function fetchContributorInfo( export async function fetchPRBranches( owner: string, repo: string, + prNumbers: number[], state: string = "open", page: number = 1, ): Promise { @@ -87,6 +88,7 @@ export async function fetchPRBranches( type: "FETCH_PR_BRANCHES", owner, repo, + prNumbers, state, page, }); diff --git a/src/lib/messages.ts b/src/lib/messages.ts index c49fec7..c3799d9 100644 --- a/src/lib/messages.ts +++ b/src/lib/messages.ts @@ -103,7 +103,7 @@ export interface ContributorInfo { } export type ServiceWorkerRequest = - | { type: "FETCH_PR_BRANCHES"; owner: string; repo: string; state: string; page: number } + | { type: "FETCH_PR_BRANCHES"; owner: string; repo: string; prNumbers: number[]; state: string; page: number } | { type: "FETCH_PR_CONFLICT_STATUSES"; owner: string; repo: string; prNumbers: number[] } | { type: "FETCH_PR_REVIEW_STATUSES"; owner: string; repo: string; prNumbers: number[] } | { type: "FETCH_PR_REVIEW_THREAD_DETAILS"; owner: string; repo: string; prNumber: number } diff --git a/src/service-worker.test.ts b/src/service-worker.test.ts index 2f88b1e..7458f62 100644 --- a/src/service-worker.test.ts +++ b/src/service-worker.test.ts @@ -130,7 +130,7 @@ describe("service worker", () => { }); it("coalesces concurrent requests and caches successful PR branch responses", async () => { - const state = await loadWorker("token"); + const state = await loadWorker(); const fetchDeferred = deferred(); const fetchMock = vi.mocked(fetch); fetchMock.mockReturnValue(fetchDeferred.promise); @@ -139,6 +139,7 @@ describe("service worker", () => { type: "FETCH_PR_BRANCHES", owner: "owner", repo: "repo", + prNumbers: [7], state: "open", page: 1, }; @@ -157,7 +158,7 @@ describe("service worker", () => { it("returns fresh cached data without fetching", async () => { vi.spyOn(Date, "now").mockReturnValue(1_000); - const state = await loadWorker("token"); + const state = await loadWorker(); state.sessionStore["cache:branches:owner/repo:open:1"] = { data: [{ number: 1, headRef: "cached" }], timestamp: 900, @@ -167,6 +168,7 @@ describe("service worker", () => { type: "FETCH_PR_BRANCHES", owner: "owner", repo: "repo", + prNumbers: [1], state: "open", page: 1, }); @@ -177,7 +179,7 @@ describe("service worker", () => { it("expires stale cache entries before fetching", async () => { vi.spyOn(Date, "now").mockReturnValue(10 * 60 * 1000); - const state = await loadWorker("token"); + const state = await loadWorker(); state.sessionStore["cache:branches:owner/repo:open:1"] = { data: [{ number: 1, headRef: "stale" }], timestamp: 0, @@ -188,6 +190,7 @@ describe("service worker", () => { type: "FETCH_PR_BRANCHES", owner: "owner", repo: "repo", + prNumbers: [2], state: "open", page: 1, }); @@ -198,6 +201,63 @@ describe("service worker", () => { }); }); + it("fetches branches by visible PR number when the list page response does not match", async () => { + const state = await loadWorker("token"); + vi.mocked(fetch).mockResolvedValueOnce( + jsonResponse({ + data: { + repository: { + pr_7: { headRefName: "feature/a" }, + pr_8: { headRefName: "fix/b" }, + }, + }, + }), + ); + + const response = await sendMessage(state.messageListeners[0], { + type: "FETCH_PR_BRANCHES", + owner: "owner", + repo: "repo", + prNumbers: [8, 7], + state: "open", + page: 1, + }); + + expect(response).toEqual({ + ok: true, + data: [ + { number: 7, headRef: "feature/a" }, + { number: 8, headRef: "fix/b" }, + ], + }); + expect(fetch).toHaveBeenCalledTimes(1); + expect(vi.mocked(fetch).mock.calls[0][0]).toBe("https://api.github.com/graphql"); + }); + + it("falls back to exact REST requests for public filtered lists", async () => { + const state = await loadWorker(); + vi.mocked(fetch) + .mockResolvedValueOnce(jsonResponse([])) + .mockResolvedValueOnce(jsonResponse({ number: 7, head: { ref: "feature/a" } })); + + const response = await sendMessage(state.messageListeners[0], { + type: "FETCH_PR_BRANCHES", + owner: "owner", + repo: "repo", + prNumbers: [7], + state: "open", + page: 1, + }); + + expect(response).toEqual({ + ok: true, + data: [{ number: 7, headRef: "feature/a" }], + }); + expect(vi.mocked(fetch).mock.calls[1][0]).toBe( + "https://api.github.com/repos/owner/repo/pulls/7", + ); + }); + it("does not call GraphQL-backed endpoints without a token", async () => { const state = await loadWorker(); diff --git a/src/service-worker.ts b/src/service-worker.ts index 39fff12..77ef8c1 100644 --- a/src/service-worker.ts +++ b/src/service-worker.ts @@ -86,14 +86,33 @@ async function fetchPRBranches( repo: string, state: string, page: number, + prNumbers: number[], ): Promise { + const requested = [...new Set(prNumbers)].sort((a, b) => a - b); + if (requested.length === 0) return []; + + const token = await getToken(); + if (token) { + return fetchGraphQLBatch({ + cachePrefix: "branches", + owner, + repo, + keys: requested, + aliasFor: (number) => `pr_${number}`, + buildNodeQuery: (number) => `pullRequest(number: ${number}) { + headRefName + }`, + parseNode: (number, pr) => + typeof pr.headRefName === "string" ? { number, headRef: pr.headRefName } : null, + }); + } + const cacheKey = `cache:branches:${owner}/${repo}:${state}:${page}`; - return cachedFetch(cacheKey, async () => { + const pageBranches = await cachedFetch(cacheKey, async () => { const perPage = 30; const url = `https://api.github.com/repos/${owner}/${repo}/pulls?state=${state}&sort=updated&direction=desc&page=${page}&per_page=${perPage}`; - const headers = restHeaders(await getToken()); - const response = await fetch(url, { headers }); + const response = await fetch(url, { headers: restHeaders(token) }); if (!response.ok) { console.error(`[Better GitHub] API error: ${response.status} ${response.statusText}`); @@ -106,6 +125,27 @@ async function fetchPRBranches( headRef: pr.head.ref, })); }); + + const missing = requested.filter( + (number) => !pageBranches.some((branch) => branch.number === number), + ); + const exactBranches = await Promise.all( + missing.map((number) => + cachedFetch(`cache:branches:${owner}/${repo}:pr:${number}`, async () => { + const response = await fetch( + `https://api.github.com/repos/${owner}/${repo}/pulls/${number}`, + { headers: restHeaders(token) }, + ); + if (!response.ok) { + console.error(`[Better GitHub] API error: ${response.status} ${response.statusText}`); + return []; + } + const pull = (await response.json()) as { number: number; head: { ref: string } }; + return [{ number: pull.number, headRef: pull.head.ref }]; + }), + ), + ); + return pageBranches.concat(...exactBranches); } interface GraphQLBatchSpec { @@ -772,7 +812,7 @@ async function handleMessage( case "FETCH_PR_BRANCHES": return { ok: true, - data: await fetchPRBranches(request.owner, request.repo, request.state, request.page), + data: await fetchPRBranches(request.owner, request.repo, request.state, request.page, request.prNumbers), }; case "FETCH_PR_CONFLICT_STATUSES": return { From d6e7e7abb05814f5d2915fc48c8063d695e11a6b Mon Sep 17 00:00:00 2001 From: shawn Date: Thu, 30 Jul 2026 11:13:13 +0800 Subject: [PATCH 2/3] refactor: share pr list dom collection --- src/features/pr-branch-names.ts | 13 ++++--------- src/features/pr-conflict-indicator.ts | 8 ++------ src/features/pr-diff-stats.test.ts | 3 +-- src/features/pr-diff-stats.ts | 17 ++++------------- src/features/pr-review-status.test.ts | 3 +-- src/features/pr-review-status.ts | 18 ++++-------------- src/lib/info-row-skeleton.ts | 3 ++- src/lib/pr-list-dom.test.ts | 20 ++++++++++++++++++++ src/lib/pr-list-dom.ts | 13 +++++++++++++ 9 files changed, 51 insertions(+), 47 deletions(-) create mode 100644 src/lib/pr-list-dom.test.ts create mode 100644 src/lib/pr-list-dom.ts diff --git a/src/features/pr-branch-names.ts b/src/features/pr-branch-names.ts index f4f759a..9dd69bd 100644 --- a/src/features/pr-branch-names.ts +++ b/src/features/pr-branch-names.ts @@ -3,6 +3,7 @@ import { fetchPRBranches } from "../lib/github-api"; import { insertInfoRowItem } from "../lib/info-row"; import { clearSkeletons } from "../lib/info-row-skeleton"; import { t } from "../lib/i18n"; +import { collectPRRows } from "../lib/pr-list-dom"; const BADGE_CLASS = "better-github-branch-badge"; const COPIED_CLASS = "better-github-branch-copied"; @@ -50,10 +51,8 @@ export async function injectPRBranchNames(): Promise { const existing = document.querySelectorAll(`.${BADGE_CLASS}`); if (existing.length > 0) return; - const prRows = document.querySelectorAll("[id^='issue_']:not([id$='_link'])"); - const prNumbers = [...prRows] - .map((row) => Number(row.id.replace("issue_", ""))) - .filter(Number.isInteger); + const prRows = collectPRRows(); + const prNumbers = [...prRows.keys()]; if (prNumbers.length === 0) return; try { @@ -64,11 +63,7 @@ export async function injectPRBranchNames(): Promise { const branchMap = new Map(branches.map((b) => [b.number, b.headRef])); - for (const row of prRows) { - const id = row.getAttribute("id"); - if (!id) continue; - - const prNumber = parseInt(id.replace("issue_", ""), 10); + for (const [prNumber, row] of prRows) { const branchName = branchMap.get(prNumber); if (!branchName) continue; diff --git a/src/features/pr-conflict-indicator.ts b/src/features/pr-conflict-indicator.ts index 7d699c2..bbc83b3 100644 --- a/src/features/pr-conflict-indicator.ts +++ b/src/features/pr-conflict-indicator.ts @@ -1,6 +1,7 @@ import { isPRListPage, getRepoInfo } from "../lib/page-detect"; import { fetchPRConflictStatuses } from "../lib/github-api"; import { insertInfoRowItem } from "../lib/info-row"; +import { collectPRRows, getPRNumber } from "../lib/pr-list-dom"; import { t } from "../lib/i18n"; const INDICATOR_CLASS = "better-github-conflict-indicator"; @@ -10,11 +11,6 @@ let observedRepo: string | null = null; let checkedRows = new WeakSet(); let generation = 0; -function getPRNumber(row: Element): number | null { - const number = Number(row.id.replace("issue_", "")); - return Number.isInteger(number) ? number : null; -} - function hasConflictLabel(row: Element): boolean { return [...row.querySelectorAll(".IssueLabel")].some((label) => /^conflicts?$/i.test( @@ -97,7 +93,7 @@ export function injectPRConflictIndicator(): void { observer = currentObserver; } - for (const row of document.querySelectorAll("[id^='issue_']:not([id$='_link'])")) { + for (const row of collectPRRows().values()) { if (!checkedRows.has(row) && !row.querySelector(`.${INDICATOR_CLASS}`)) { observer.observe(row); } diff --git a/src/features/pr-diff-stats.test.ts b/src/features/pr-diff-stats.test.ts index 8a7bc3b..dee9660 100644 --- a/src/features/pr-diff-stats.test.ts +++ b/src/features/pr-diff-stats.test.ts @@ -41,8 +41,7 @@ describe("injectPRDiffStats", () => { const [owner, repo, numbers] = vi.mocked(fetchPRDiffStats).mock.calls[0]; expect(owner).toBe("owner"); expect(repo).toBe("repo"); - expect(numbers).toContain(7); - expect(numbers).toContain(8); + expect(numbers).toEqual([7, 8]); const badge7 = document.querySelector("#issue_7 .better-github-diff-stats") as HTMLElement; expect(badge7).not.toBeNull(); diff --git a/src/features/pr-diff-stats.ts b/src/features/pr-diff-stats.ts index 6181f75..7fb1724 100644 --- a/src/features/pr-diff-stats.ts +++ b/src/features/pr-diff-stats.ts @@ -3,6 +3,7 @@ import { fetchPRDiffStats } from "../lib/github-api"; import { insertInfoRowItem } from "../lib/info-row"; import { buildDiffStatsBadge } from "../lib/diff-stats-badge"; import { clearSkeletons } from "../lib/info-row-skeleton"; +import { collectPRRows } from "../lib/pr-list-dom"; const BADGE_CLASS = "better-github-diff-stats"; @@ -12,14 +13,8 @@ export async function injectPRDiffStats(): Promise { const info = getRepoInfo(); if (!info) return; - const prRows = document.querySelectorAll("[id^='issue_']"); - const prNumbers: number[] = []; - for (const row of prRows) { - const id = row.getAttribute("id"); - if (!id) continue; - prNumbers.push(parseInt(id.replace("issue_", ""), 10)); - } - + const prRows = collectPRRows(); + const prNumbers = [...prRows.keys()]; if (prNumbers.length === 0) return; try { @@ -28,11 +23,7 @@ export async function injectPRDiffStats(): Promise { const statsMap = new Map(stats.map((s) => [s.number, s])); - for (const row of prRows) { - const id = row.getAttribute("id"); - if (!id) continue; - - const prNumber = parseInt(id.replace("issue_", ""), 10); + for (const [prNumber, row] of prRows) { const stat = statsMap.get(prNumber); if (!stat) continue; diff --git a/src/features/pr-review-status.test.ts b/src/features/pr-review-status.test.ts index a728d64..349633c 100644 --- a/src/features/pr-review-status.test.ts +++ b/src/features/pr-review-status.test.ts @@ -49,8 +49,7 @@ describe("injectPRReviewStatus", () => { const [owner, repo, numbers] = vi.mocked(fetchPRReviewStatuses).mock.calls[0]; expect(owner).toBe("owner"); expect(repo).toBe("repo"); - expect(numbers).toContain(7); - expect(numbers).toContain(8); + expect(numbers).toEqual([7, 8]); // All resolved → check-marked "All resolved" state, with a simple tooltip // (no popover, so no overlap risk). diff --git a/src/features/pr-review-status.ts b/src/features/pr-review-status.ts index 767ec93..909bfa2 100644 --- a/src/features/pr-review-status.ts +++ b/src/features/pr-review-status.ts @@ -2,6 +2,7 @@ import { isPRListPage, getRepoInfo } from "../lib/page-detect"; import { fetchPRReviewStatuses, fetchReviewThreadDetails } from "../lib/github-api"; import type { ReviewThreadDetail } from "../lib/messages"; import { insertInfoRowItem } from "../lib/info-row"; +import { collectPRRows } from "../lib/pr-list-dom"; // Aliased to `i18n` because this module already uses `t` as a thread loop var. import { t as i18n } from "../lib/i18n"; @@ -249,15 +250,8 @@ export async function injectPRReviewStatus(): Promise { // Skip if already injected if (document.querySelectorAll(`.${STATUS_CLASS}`).length > 0) return; - // Collect PR numbers from the page - const prRows = document.querySelectorAll("[id^='issue_']"); - const prNumbers: number[] = []; - for (const row of prRows) { - const id = row.getAttribute("id"); - if (!id) continue; - prNumbers.push(parseInt(id.replace("issue_", ""), 10)); - } - + const prRows = collectPRRows(); + const prNumbers = [...prRows.keys()]; if (prNumbers.length === 0) return; const statuses = await fetchPRReviewStatuses(info.owner, info.repo, prNumbers); @@ -266,11 +260,7 @@ export async function injectPRReviewStatus(): Promise { const statusMap = new Map(statuses.map((s) => [s.number, s])); - for (const row of prRows) { - const id = row.getAttribute("id"); - if (!id) continue; - - const prNumber = parseInt(id.replace("issue_", ""), 10); + for (const [prNumber, row] of prRows) { const status = statusMap.get(prNumber); if (!status || status.totalThreads === 0) continue; diff --git a/src/lib/info-row-skeleton.ts b/src/lib/info-row-skeleton.ts index 41a066a..77fd071 100644 --- a/src/lib/info-row-skeleton.ts +++ b/src/lib/info-row-skeleton.ts @@ -1,6 +1,7 @@ import { isPRListPage, isCommitsListPage, getRepoInfo } from "./page-detect"; import { collectCommitRows, MAIN_CONTENT_INNER_SELECTOR } from "./commit-dom"; import { insertInfoRowItem } from "./info-row"; +import { collectPRRows } from "./pr-list-dom"; export type SkeletonKind = "branch" | "prDiff" | "commitDiff"; @@ -53,7 +54,7 @@ function reservePRListSkeletons(flags: SkeletonFlags): void { .map((c) => `.${c}`) .join(", "); - for (const row of document.querySelectorAll("[id^='issue_']")) { + for (const row of collectPRRows().values()) { const present = new Set( [...row.querySelectorAll(probeSelector)].flatMap((el) => [...el.classList]), ); diff --git a/src/lib/pr-list-dom.test.ts b/src/lib/pr-list-dom.test.ts new file mode 100644 index 0000000..dc51407 --- /dev/null +++ b/src/lib/pr-list-dom.test.ts @@ -0,0 +1,20 @@ +import { beforeEach, describe, expect, it } from "vitest"; +import { collectPRRows, getPRNumber } from "./pr-list-dom"; + +describe("PR list DOM helpers", () => { + beforeEach(() => { + document.body.innerHTML = ` + +
+
+ `; + }); + + it("collects each PR row once and ignores title links", () => { + const rows = collectPRRows(); + + expect([...rows.keys()]).toEqual([7, 8]); + expect(rows.get(7)).toBe(document.getElementById("issue_7")); + expect(getPRNumber(document.getElementById("issue_7_link")!)).toBeNull(); + }); +}); diff --git a/src/lib/pr-list-dom.ts b/src/lib/pr-list-dom.ts new file mode 100644 index 0000000..5a7354d --- /dev/null +++ b/src/lib/pr-list-dom.ts @@ -0,0 +1,13 @@ +export function getPRNumber(row: Element): number | null { + const match = /^issue_(\d+)$/.exec(row.id); + return match ? Number(match[1]) : null; +} + +export function collectPRRows(): Map { + const rows = new Map(); + for (const row of document.querySelectorAll("[id^='issue_']:not([id$='_link'])")) { + const number = getPRNumber(row); + if (number !== null) rows.set(number, row); + } + return rows; +} From 3a47b9fdb99d7246433081e112b885f3b63cdcf9 Mon Sep 17 00:00:00 2001 From: shawn Date: Thu, 30 Jul 2026 11:28:46 +0800 Subject: [PATCH 3/3] refactor: revert shared pr list dom collection This reverts commit d6e7e7abb05814f5d2915fc48c8063d695e11a6b. --- src/features/pr-branch-names.ts | 13 +++++++++---- src/features/pr-conflict-indicator.ts | 8 ++++++-- src/features/pr-diff-stats.test.ts | 3 ++- src/features/pr-diff-stats.ts | 17 +++++++++++++---- src/features/pr-review-status.test.ts | 3 ++- src/features/pr-review-status.ts | 18 ++++++++++++++---- src/lib/info-row-skeleton.ts | 3 +-- src/lib/pr-list-dom.test.ts | 20 -------------------- src/lib/pr-list-dom.ts | 13 ------------- 9 files changed, 47 insertions(+), 51 deletions(-) delete mode 100644 src/lib/pr-list-dom.test.ts delete mode 100644 src/lib/pr-list-dom.ts diff --git a/src/features/pr-branch-names.ts b/src/features/pr-branch-names.ts index 9dd69bd..f4f759a 100644 --- a/src/features/pr-branch-names.ts +++ b/src/features/pr-branch-names.ts @@ -3,7 +3,6 @@ import { fetchPRBranches } from "../lib/github-api"; import { insertInfoRowItem } from "../lib/info-row"; import { clearSkeletons } from "../lib/info-row-skeleton"; import { t } from "../lib/i18n"; -import { collectPRRows } from "../lib/pr-list-dom"; const BADGE_CLASS = "better-github-branch-badge"; const COPIED_CLASS = "better-github-branch-copied"; @@ -51,8 +50,10 @@ export async function injectPRBranchNames(): Promise { const existing = document.querySelectorAll(`.${BADGE_CLASS}`); if (existing.length > 0) return; - const prRows = collectPRRows(); - const prNumbers = [...prRows.keys()]; + const prRows = document.querySelectorAll("[id^='issue_']:not([id$='_link'])"); + const prNumbers = [...prRows] + .map((row) => Number(row.id.replace("issue_", ""))) + .filter(Number.isInteger); if (prNumbers.length === 0) return; try { @@ -63,7 +64,11 @@ export async function injectPRBranchNames(): Promise { const branchMap = new Map(branches.map((b) => [b.number, b.headRef])); - for (const [prNumber, row] of prRows) { + for (const row of prRows) { + const id = row.getAttribute("id"); + if (!id) continue; + + const prNumber = parseInt(id.replace("issue_", ""), 10); const branchName = branchMap.get(prNumber); if (!branchName) continue; diff --git a/src/features/pr-conflict-indicator.ts b/src/features/pr-conflict-indicator.ts index bbc83b3..7d699c2 100644 --- a/src/features/pr-conflict-indicator.ts +++ b/src/features/pr-conflict-indicator.ts @@ -1,7 +1,6 @@ import { isPRListPage, getRepoInfo } from "../lib/page-detect"; import { fetchPRConflictStatuses } from "../lib/github-api"; import { insertInfoRowItem } from "../lib/info-row"; -import { collectPRRows, getPRNumber } from "../lib/pr-list-dom"; import { t } from "../lib/i18n"; const INDICATOR_CLASS = "better-github-conflict-indicator"; @@ -11,6 +10,11 @@ let observedRepo: string | null = null; let checkedRows = new WeakSet(); let generation = 0; +function getPRNumber(row: Element): number | null { + const number = Number(row.id.replace("issue_", "")); + return Number.isInteger(number) ? number : null; +} + function hasConflictLabel(row: Element): boolean { return [...row.querySelectorAll(".IssueLabel")].some((label) => /^conflicts?$/i.test( @@ -93,7 +97,7 @@ export function injectPRConflictIndicator(): void { observer = currentObserver; } - for (const row of collectPRRows().values()) { + for (const row of document.querySelectorAll("[id^='issue_']:not([id$='_link'])")) { if (!checkedRows.has(row) && !row.querySelector(`.${INDICATOR_CLASS}`)) { observer.observe(row); } diff --git a/src/features/pr-diff-stats.test.ts b/src/features/pr-diff-stats.test.ts index dee9660..8a7bc3b 100644 --- a/src/features/pr-diff-stats.test.ts +++ b/src/features/pr-diff-stats.test.ts @@ -41,7 +41,8 @@ describe("injectPRDiffStats", () => { const [owner, repo, numbers] = vi.mocked(fetchPRDiffStats).mock.calls[0]; expect(owner).toBe("owner"); expect(repo).toBe("repo"); - expect(numbers).toEqual([7, 8]); + expect(numbers).toContain(7); + expect(numbers).toContain(8); const badge7 = document.querySelector("#issue_7 .better-github-diff-stats") as HTMLElement; expect(badge7).not.toBeNull(); diff --git a/src/features/pr-diff-stats.ts b/src/features/pr-diff-stats.ts index 7fb1724..6181f75 100644 --- a/src/features/pr-diff-stats.ts +++ b/src/features/pr-diff-stats.ts @@ -3,7 +3,6 @@ import { fetchPRDiffStats } from "../lib/github-api"; import { insertInfoRowItem } from "../lib/info-row"; import { buildDiffStatsBadge } from "../lib/diff-stats-badge"; import { clearSkeletons } from "../lib/info-row-skeleton"; -import { collectPRRows } from "../lib/pr-list-dom"; const BADGE_CLASS = "better-github-diff-stats"; @@ -13,8 +12,14 @@ export async function injectPRDiffStats(): Promise { const info = getRepoInfo(); if (!info) return; - const prRows = collectPRRows(); - const prNumbers = [...prRows.keys()]; + const prRows = document.querySelectorAll("[id^='issue_']"); + const prNumbers: number[] = []; + for (const row of prRows) { + const id = row.getAttribute("id"); + if (!id) continue; + prNumbers.push(parseInt(id.replace("issue_", ""), 10)); + } + if (prNumbers.length === 0) return; try { @@ -23,7 +28,11 @@ export async function injectPRDiffStats(): Promise { const statsMap = new Map(stats.map((s) => [s.number, s])); - for (const [prNumber, row] of prRows) { + for (const row of prRows) { + const id = row.getAttribute("id"); + if (!id) continue; + + const prNumber = parseInt(id.replace("issue_", ""), 10); const stat = statsMap.get(prNumber); if (!stat) continue; diff --git a/src/features/pr-review-status.test.ts b/src/features/pr-review-status.test.ts index 349633c..a728d64 100644 --- a/src/features/pr-review-status.test.ts +++ b/src/features/pr-review-status.test.ts @@ -49,7 +49,8 @@ describe("injectPRReviewStatus", () => { const [owner, repo, numbers] = vi.mocked(fetchPRReviewStatuses).mock.calls[0]; expect(owner).toBe("owner"); expect(repo).toBe("repo"); - expect(numbers).toEqual([7, 8]); + expect(numbers).toContain(7); + expect(numbers).toContain(8); // All resolved → check-marked "All resolved" state, with a simple tooltip // (no popover, so no overlap risk). diff --git a/src/features/pr-review-status.ts b/src/features/pr-review-status.ts index 909bfa2..767ec93 100644 --- a/src/features/pr-review-status.ts +++ b/src/features/pr-review-status.ts @@ -2,7 +2,6 @@ import { isPRListPage, getRepoInfo } from "../lib/page-detect"; import { fetchPRReviewStatuses, fetchReviewThreadDetails } from "../lib/github-api"; import type { ReviewThreadDetail } from "../lib/messages"; import { insertInfoRowItem } from "../lib/info-row"; -import { collectPRRows } from "../lib/pr-list-dom"; // Aliased to `i18n` because this module already uses `t` as a thread loop var. import { t as i18n } from "../lib/i18n"; @@ -250,8 +249,15 @@ export async function injectPRReviewStatus(): Promise { // Skip if already injected if (document.querySelectorAll(`.${STATUS_CLASS}`).length > 0) return; - const prRows = collectPRRows(); - const prNumbers = [...prRows.keys()]; + // Collect PR numbers from the page + const prRows = document.querySelectorAll("[id^='issue_']"); + const prNumbers: number[] = []; + for (const row of prRows) { + const id = row.getAttribute("id"); + if (!id) continue; + prNumbers.push(parseInt(id.replace("issue_", ""), 10)); + } + if (prNumbers.length === 0) return; const statuses = await fetchPRReviewStatuses(info.owner, info.repo, prNumbers); @@ -260,7 +266,11 @@ export async function injectPRReviewStatus(): Promise { const statusMap = new Map(statuses.map((s) => [s.number, s])); - for (const [prNumber, row] of prRows) { + for (const row of prRows) { + const id = row.getAttribute("id"); + if (!id) continue; + + const prNumber = parseInt(id.replace("issue_", ""), 10); const status = statusMap.get(prNumber); if (!status || status.totalThreads === 0) continue; diff --git a/src/lib/info-row-skeleton.ts b/src/lib/info-row-skeleton.ts index 77fd071..41a066a 100644 --- a/src/lib/info-row-skeleton.ts +++ b/src/lib/info-row-skeleton.ts @@ -1,7 +1,6 @@ import { isPRListPage, isCommitsListPage, getRepoInfo } from "./page-detect"; import { collectCommitRows, MAIN_CONTENT_INNER_SELECTOR } from "./commit-dom"; import { insertInfoRowItem } from "./info-row"; -import { collectPRRows } from "./pr-list-dom"; export type SkeletonKind = "branch" | "prDiff" | "commitDiff"; @@ -54,7 +53,7 @@ function reservePRListSkeletons(flags: SkeletonFlags): void { .map((c) => `.${c}`) .join(", "); - for (const row of collectPRRows().values()) { + for (const row of document.querySelectorAll("[id^='issue_']")) { const present = new Set( [...row.querySelectorAll(probeSelector)].flatMap((el) => [...el.classList]), ); diff --git a/src/lib/pr-list-dom.test.ts b/src/lib/pr-list-dom.test.ts deleted file mode 100644 index dc51407..0000000 --- a/src/lib/pr-list-dom.test.ts +++ /dev/null @@ -1,20 +0,0 @@ -import { beforeEach, describe, expect, it } from "vitest"; -import { collectPRRows, getPRNumber } from "./pr-list-dom"; - -describe("PR list DOM helpers", () => { - beforeEach(() => { - document.body.innerHTML = ` - -
-
- `; - }); - - it("collects each PR row once and ignores title links", () => { - const rows = collectPRRows(); - - expect([...rows.keys()]).toEqual([7, 8]); - expect(rows.get(7)).toBe(document.getElementById("issue_7")); - expect(getPRNumber(document.getElementById("issue_7_link")!)).toBeNull(); - }); -}); diff --git a/src/lib/pr-list-dom.ts b/src/lib/pr-list-dom.ts deleted file mode 100644 index 5a7354d..0000000 --- a/src/lib/pr-list-dom.ts +++ /dev/null @@ -1,13 +0,0 @@ -export function getPRNumber(row: Element): number | null { - const match = /^issue_(\d+)$/.exec(row.id); - return match ? Number(match[1]) : null; -} - -export function collectPRRows(): Map { - const rows = new Map(); - for (const row of document.querySelectorAll("[id^='issue_']:not([id$='_link'])")) { - const number = getPRNumber(row); - if (number !== null) rows.set(number, row); - } - return rows; -}