Skip to content

Commit 36b94bf

Browse files
kylecarbskylecarbs
committed
Optimisically update the PR UI when changes are made
1 parent 079c207 commit 36b94bf

File tree

4 files changed

+264
-59
lines changed

4 files changed

+264
-59
lines changed

‎AGENTS.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ Pulldash is the fastest way to review pull requests.
66

77
Use `bun` for everything - package management, tests.
88

9+
_NEVER_ use browser tools - they will not work with this project.
10+
911
## Principles
1012

1113
- Performance is P1. If things feel laggy or are not smooth, it is of the utmost importance to fix. Pulldash exists because GitHub's PR review is slow.

‎src/browser/components/pr-overview.tsx‎

Lines changed: 198 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -399,66 +399,204 @@ export const PROverview = memo(function PROverview() {
399399
await store.restoreBranch();
400400
}, [store]);
401401

402+
// Helper to refetch timeline after mutations
403+
const refetchTimeline = useCallback(() => {
404+
github
405+
.getPRTimeline(owner, repo, pr.number)
406+
.then((timeline) => store.setTimeline(timeline))
407+
.catch(() => {});
408+
}, [github, owner, repo, pr.number, store]);
409+
402410
const handleRequestReviewer = useCallback(
403411
async (login: string) => {
412+
// Find the collaborator to get avatar_url
413+
const collaborator = collaborators.find((c) => c.login === login);
404414
try {
415+
// 1. Request GitHub to add reviewer
405416
await github.requestReviewers(owner, repo, pr.number, [login]);
406-
await refetchPR();
417+
418+
// 2. Update our state with the known change
419+
const newReviewer = {
420+
login,
421+
avatar_url: collaborator?.avatar_url ?? "",
422+
id: 0,
423+
node_id: "",
424+
gravatar_id: "",
425+
url: "",
426+
html_url: "",
427+
followers_url: "",
428+
following_url: "",
429+
gists_url: "",
430+
starred_url: "",
431+
subscriptions_url: "",
432+
organizations_url: "",
433+
repos_url: "",
434+
events_url: "",
435+
received_events_url: "",
436+
type: "User" as const,
437+
site_admin: false,
438+
user_view_type: "public" as const,
439+
};
440+
store.setPr({
441+
...pr,
442+
requested_reviewers: [...(pr.requested_reviewers ?? []), newReviewer],
443+
});
444+
445+
// 3. Invalidate cache so future fetches get fresh data
446+
github.invalidateCache(`pr:${owner}/${repo}/${pr.number}`);
447+
448+
// 4. Refetch timeline (reviewer request creates event)
449+
refetchTimeline();
407450
} catch (error) {
408451
console.error("Failed to request reviewer:", error);
409452
}
410453
},
411-
[github, owner, repo, pr.number, refetchPR]
454+
[github, owner, repo, pr, store, collaborators, refetchTimeline]
412455
);
413456

414457
const handleRemoveReviewer = useCallback(
415458
async (login: string) => {
416459
try {
460+
// 1. Request GitHub to remove reviewer
417461
await github.removeReviewers(owner, repo, pr.number, [login]);
418-
await refetchPR();
462+
463+
// 2. Update our state with the known change
464+
store.setPr({
465+
...pr,
466+
requested_reviewers: (pr.requested_reviewers ?? []).filter(
467+
(r) => r.login !== login
468+
),
469+
});
470+
471+
// 3. Invalidate cache so future fetches get fresh data
472+
github.invalidateCache(`pr:${owner}/${repo}/${pr.number}`);
473+
474+
// 4. Refetch timeline
475+
refetchTimeline();
419476
} catch (error) {
420477
console.error("Failed to remove reviewer:", error);
421478
}
422479
},
423-
[github, owner, repo, pr.number, refetchPR]
480+
[github, owner, repo, pr, store, refetchTimeline]
424481
);
425482

426483
const handleAddAssignee = useCallback(
427484
async (login: string) => {
485+
// Find the collaborator to get avatar_url
486+
const collaborator = collaborators.find((c) => c.login === login);
428487
try {
488+
// 1. Request GitHub to add assignee
429489
await github.addAssignees(owner, repo, pr.number, [login]);
430-
await refetchPR();
490+
491+
// 2. Update our state with the known change
492+
const newAssignee = {
493+
login,
494+
avatar_url: collaborator?.avatar_url ?? "",
495+
id: 0,
496+
node_id: "",
497+
gravatar_id: "",
498+
url: "",
499+
html_url: "",
500+
followers_url: "",
501+
following_url: "",
502+
gists_url: "",
503+
starred_url: "",
504+
subscriptions_url: "",
505+
organizations_url: "",
506+
repos_url: "",
507+
events_url: "",
508+
received_events_url: "",
509+
type: "User" as const,
510+
site_admin: false,
511+
user_view_type: "public" as const,
512+
};
513+
store.setPr({
514+
...pr,
515+
assignees: [...(pr.assignees ?? []), newAssignee],
516+
});
517+
518+
// 3. Invalidate cache so future fetches get fresh data
519+
github.invalidateCache(`pr:${owner}/${repo}/${pr.number}`);
520+
521+
// 4. Refetch timeline (assignee change creates event)
522+
refetchTimeline();
431523
} catch (error) {
432524
console.error("Failed to add assignee:", error);
433525
}
434526
},
435-
[github, owner, repo, pr.number, refetchPR]
527+
[github, owner, repo, pr, store, collaborators, refetchTimeline]
436528
);
437529

438530
const handleRemoveAssignee = useCallback(
439531
async (login: string) => {
440532
try {
533+
// 1. Request GitHub to remove assignee
441534
await github.removeAssignees(owner, repo, pr.number, [login]);
442-
await refetchPR();
535+
536+
// 2. Update our state with the known change
537+
store.setPr({
538+
...pr,
539+
assignees: (pr.assignees ?? []).filter((a) => a.login !== login),
540+
});
541+
542+
// 3. Invalidate cache so future fetches get fresh data
543+
github.invalidateCache(`pr:${owner}/${repo}/${pr.number}`);
544+
545+
// 4. Refetch timeline
546+
refetchTimeline();
443547
} catch (error) {
444548
console.error("Failed to remove assignee:", error);
445549
}
446550
},
447-
[github, owner, repo, pr.number, refetchPR]
551+
[github, owner, repo, pr, store, refetchTimeline]
448552
);
449553

450554
const handleAssignSelf = useCallback(async () => {
451555
if (!currentUser) return;
556+
452557
setAssigningSelf(true);
453558
try {
559+
// 1. Request GitHub to add assignee
454560
await github.addAssignees(owner, repo, pr.number, [currentUser]);
455-
await refetchPR();
561+
562+
// 2. Update our state with the known change
563+
const newAssignee = {
564+
login: currentUser,
565+
avatar_url: "",
566+
id: 0,
567+
node_id: "",
568+
gravatar_id: "",
569+
url: "",
570+
html_url: "",
571+
followers_url: "",
572+
following_url: "",
573+
gists_url: "",
574+
starred_url: "",
575+
subscriptions_url: "",
576+
organizations_url: "",
577+
repos_url: "",
578+
events_url: "",
579+
received_events_url: "",
580+
type: "User" as const,
581+
site_admin: false,
582+
user_view_type: "public" as const,
583+
};
584+
store.setPr({
585+
...pr,
586+
assignees: [...(pr.assignees ?? []), newAssignee],
587+
});
588+
589+
// 3. Invalidate cache so future fetches get fresh data
590+
github.invalidateCache(`pr:${owner}/${repo}/${pr.number}`);
591+
592+
// 4. Refetch timeline
593+
refetchTimeline();
456594
} catch (error) {
457595
console.error("Failed to assign self:", error);
458596
} finally {
459597
setAssigningSelf(false);
460598
}
461-
}, [github, owner, repo, pr.number, currentUser, refetchPR]);
599+
}, [github, owner, repo, pr, currentUser, store, refetchTimeline]);
462600

463601
// Reaction state - keyed by "issue" for PR body or comment ID
464602
const [reactions, setReactions] = useState<Record<string, Reaction[]>>({});
@@ -1639,7 +1777,44 @@ export const PROverview = memo(function PROverview() {
16391777
pr={pr}
16401778
owner={owner}
16411779
repo={repo}
1642-
onUpdate={refetchPR}
1780+
onLabelToggle={async (labelName, labelColor, hasLabel) => {
1781+
try {
1782+
// 1. Request GitHub to toggle label
1783+
if (hasLabel) {
1784+
await github.removeLabel(owner, repo, pr.number, labelName);
1785+
} else {
1786+
await github.addLabels(owner, repo, pr.number, [labelName]);
1787+
}
1788+
1789+
// 2. Update our state with the known change
1790+
const newLabels = hasLabel
1791+
? pr.labels.filter((l) => l.name !== labelName)
1792+
: [
1793+
...pr.labels,
1794+
{
1795+
id: 0,
1796+
node_id: "",
1797+
url: "",
1798+
name: labelName,
1799+
color: labelColor,
1800+
default: false,
1801+
description: null,
1802+
},
1803+
];
1804+
store.setPr({ ...pr, labels: newLabels });
1805+
1806+
// 3. Invalidate cache so future fetches get fresh data
1807+
github.invalidateCache(`pr:${owner}/${repo}/${pr.number}`);
1808+
1809+
// 4. Refetch timeline (label change creates event)
1810+
github
1811+
.getPRTimeline(owner, repo, pr.number)
1812+
.then((timeline) => store.setTimeline(timeline))
1813+
.catch(() => {});
1814+
} catch (error) {
1815+
console.error("Failed to toggle label:", error);
1816+
}
1817+
}}
16431818
canWrite={canMergeRepo}
16441819
/>
16451820

@@ -1754,23 +1929,22 @@ function SidebarSection({
17541929
// ============================================================================
17551930

17561931
interface LabelsSectionProps {
1757-
pr: {
1758-
labels: Array<{ name: string; color: string }>;
1759-
state: string;
1760-
merged?: boolean;
1761-
number: number;
1762-
};
1932+
pr: PullRequest;
17631933
owner: string;
17641934
repo: string;
1765-
onUpdate: () => Promise<void>;
1935+
onLabelToggle: (
1936+
labelName: string,
1937+
labelColor: string,
1938+
hasLabel: boolean
1939+
) => Promise<void>;
17661940
canWrite?: boolean;
17671941
}
17681942

17691943
function LabelsSection({
17701944
pr,
17711945
owner,
17721946
repo,
1773-
onUpdate,
1947+
onLabelToggle,
17741948
canWrite = true,
17751949
}: LabelsSectionProps) {
17761950
const github = useGitHub();
@@ -1808,20 +1982,11 @@ function LabelsSection({
18081982
}, [showPicker, fetchLabels]);
18091983

18101984
const handleToggleLabel = useCallback(
1811-
async (labelName: string) => {
1812-
try {
1813-
const hasLabel = pr.labels.some((l) => l.name === labelName);
1814-
if (hasLabel) {
1815-
await github.removeLabel(owner, repo, pr.number, labelName);
1816-
} else {
1817-
await github.addLabels(owner, repo, pr.number, [labelName]);
1818-
}
1819-
await onUpdate();
1820-
} catch (error) {
1821-
console.error("Failed to toggle label:", error);
1822-
}
1985+
async (labelName: string, labelColor: string) => {
1986+
const hasLabel = pr.labels.some((l) => l.name === labelName);
1987+
await onLabelToggle(labelName, labelColor, hasLabel);
18231988
},
1824-
[github, owner, repo, pr.number, pr.labels, onUpdate]
1989+
[pr.labels, onLabelToggle]
18251990
);
18261991

18271992
const canEdit = canWrite && pr.state === "open" && !pr.merged;
@@ -1889,7 +2054,7 @@ function LabelsSection({
18892054
return (
18902055
<button
18912056
key={label.name}
1892-
onClick={() => handleToggleLabel(label.name)}
2057+
onClick={() => handleToggleLabel(label.name, label.color)}
18932058
className="w-full flex items-center gap-2 px-3 py-2 hover:bg-muted transition-colors text-left"
18942059
>
18952060
<div className="w-4 h-4 flex items-center justify-center">

‎src/browser/contexts/github.tsx‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,8 @@ class RequestCache {
266266
if (!pattern) {
267267
// Clear all memory cache
268268
this.cache.clear();
269+
// Clear all pending requests
270+
this.pending.clear();
269271
// Clear persisted cache
270272
for (const key of this.persistKeys) {
271273
this.removeFromStorage(key);
@@ -278,6 +280,12 @@ class RequestCache {
278280
this.removeFromStorage(key);
279281
}
280282
}
283+
// Also clear pending requests matching the pattern
284+
for (const key of this.pending.keys()) {
285+
if (key.includes(pattern)) {
286+
this.pending.delete(key);
287+
}
288+
}
281289
}
282290

283291
/**

0 commit comments

Comments
 (0)