Make opening a PR overview fast

src/common/utils.ts

@@ -1144,13 +1144,21 @@ export async function processDiffLinks(
 	repoOwner: string,
 	repoName: string,
 	authority: string,
-	hashMap: Record<string, string>,
+	hashMap: Record<string, string> | (() => Promise<Record<string, string>>),
 	prNumber: number
 ): Promise<string> {
 	try {
 		const escapedRepoName = escapeRegExp(repoName);
 		const escapedRepoOwner = escapeRegExp(repoOwner);
 		const escapedAuthority = escapeRegExp(authority);
+		let hashMapPromise: Promise<Record<string, string>> | undefined;
+		const getHashMap = () => {
+			if (typeof hashMap !== 'function') {
+				return Promise.resolve(hashMap);
+			}
+			hashMapPromise ??= hashMap();
+			return hashMapPromise;
+		};
 
 		const diffPattern = new RegExp(
 			`<a\\s+(?![^>]*data-permalink-processed)([^>]*?href="https?:\/\/${escapedAuthority}\/${escapedRepoOwner}\/${escapedRepoName}\/pull\/${prNumber}\/(?:files|changes)#diff-(?<diffHash>[a-f0-9]{64})(?:R(?<startLine>\\d+)(?:-R(?<endLine>\\d+))?)?"[^>]*?)>(?<linkText>[^<]*?)<\/a>`,
@@ -1171,7 +1179,7 @@ export async function processDiffLinks(
 				const originalUrl = hrefMatch ? hrefMatch[1] : '';
 
 				// Look up filename from hash
-				const fileName = hashMap[diffHash];
+				const fileName = (await getHashMap())[diffHash];
 				if (fileName) {
 					// Hash found - add data attributes for diff handling and "(view on GitHub)" suffix
 					const startLineValue = startLine || '1';

src/github/issueOverview.ts

@@ -257,6 +257,10 @@ export class IssueOverviewPanel<TItem extends IssueModel = IssueModel> extends W
 			...label,
 			displayName: emojify(label.name)
 		}));
+		const [bodyHTML, events] = await Promise.all([
+			this.processLinksInBodyHtml(issue.bodyHTML),
+			this.processTimelineEvents(timelineEvents),
+		]);
 
 		const context: Issue = {
 			owner: issue.remote.owner,
@@ -267,12 +271,12 @@ export class IssueOverviewPanel<TItem extends IssueModel = IssueModel> extends W
 			url: issue.html_url,
 			createdAt: issue.createdAt,
 			body: issue.body,
-			bodyHTML: await this.processLinksInBodyHtml(issue.bodyHTML),
+			bodyHTML,
 			labels: labels,
 			author: issue.author,
 			state: issue.state,
 			stateReason: issue.stateReason,
-			events: await this.processTimelineEvents(timelineEvents),
+			events,
 			continueOnGitHub: this.continueOnGitHub(),
 			canEdit,
 			hasWritePermission,

src/github/utils.ts

@@ -389,7 +389,7 @@ export async function processPermalinks(
 export async function processDiffLinks(
 	bodyHTML: string,
 	githubRepository: GitHubRepository,
-	hashMap: Record<string, string>,
+	hashMap: Record<string, string> | (() => Promise<Record<string, string>>),
 	prNumber: number
 ): Promise<string> {
 	try {

src/test/common/utils.test.ts

@@ -207,6 +207,18 @@ describe('utils', () => {
 			assert.strictEqual(result, html);
 		});
 
+		it('should not resolve a lazy hash map for non-diff links', async () => {
+			let callCount = 0;
+			const html = '<a href="https://example.com">example</a>';
+			const result = await utils.processDiffLinks(html, repoOwner, repoName, authority, async () => {
+				callCount++;
+				return { [diffHash]: 'src/file.ts' };
+			}, prNumber);
+
+			assert.strictEqual(result, html);
+			assert.strictEqual(callCount, 0);
+		});
+
 		it('should not modify links to a different repo', async () => {
 			const hashMap: Record<string, string> = { [diffHash]: 'src/file.ts' };
 			const html = `<a href="https://github.com/other/repo/pull/${prNumber}/files#diff-${diffHash}R10">link</a>`;
@@ -242,6 +254,18 @@ describe('utils', () => {
 			assert(!result.includes('data-local-file="src/other.ts"'));
 		});
 
+		it('should resolve a lazy hash map once for multiple links', async () => {
+			let callCount = 0;
+			const html = makeDiffLink(diffHash, 1) + makeDiffLink(diffHash, 2);
+			const result = await utils.processDiffLinks(html, repoOwner, repoName, authority, async () => {
+				callCount++;
+				return { [diffHash]: 'src/found.ts' };
+			}, prNumber);
+
+			assert.strictEqual(result.match(/data-local-file="src\/found\.ts"/g)?.length, 2);
+			assert.strictEqual(callCount, 1);
+		});
+
 		it('should escape HTML special characters in file names', async () => {
 			const hashMap: Record<string, string> = { [diffHash]: 'src/file&name"test.ts' };
 			const html = makeDiffLink(diffHash, 10);

src/test/github/pullRequestOverview.test.ts

@@ -27,6 +27,7 @@ import { CheckState, GithubItemStateEnum } from '../../github/interface';
 import { CreatePullRequestHelper } from '../../view/createPullRequestHelper';
 import { RepositoriesManager } from '../../github/repositoriesManager';
 import { MockThemeWatcher } from '../mocks/mockThemeWatcher';
+import { TimelineEvent } from '../../common/timelineEvent';
 
 const EXTENSION_URI = vscode.Uri.joinPath(vscode.Uri.file(__dirname), '../../..');
 
@@ -161,6 +162,77 @@ describe('PullRequestOverview', function () {
 			assert.strictEqual(createWebviewPanel.callCount, 1);
 		});
 
+		it('coalesces an update requested during initialization', async function () {
+			const firstItem = convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(1000).title('Initial title').build(), repo);
+			const firstModel = new PullRequestModel(credentialStore, telemetry, repo, remote, firstItem);
+			const updatedItem = convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(1000).title('Updated title').build(), repo);
+			const updatedModel = new PullRequestModel(credentialStore, telemetry, repo, remote, updatedItem);
+			const identity = { owner: firstModel.remote.owner, repo: firstModel.remote.repositoryName, number: firstModel.number };
+			let releaseInitialization: (defaultBranch: string) => void;
+			const blockedInitialization = new Promise<string>(resolve => releaseInitialization = resolve);
+			sinon.stub(pullRequestManager, 'getPullRequestRepositoryDefaultBranch')
+				.onFirstCall().returns(blockedInitialization)
+				.onSecondCall().resolves('main');
+			for (const model of [firstModel, updatedModel]) {
+				sinon.stub(model, 'getReviewRequests').resolves([]);
+				sinon.stub(model, 'getTimelineEvents').resolves([]);
+				sinon.stub(model, 'validateDraftMode').resolves(false);
+				sinon.stub(model, 'getStatusChecks').resolves([{ state: CheckState.Success, statuses: [] }, null]);
+			}
+
+			const initialOpen = PullRequestOverviewPanel.createOrShow(telemetry, EXTENSION_URI, pullRequestManager, identity, firstModel);
+			await new Promise(resolve => setImmediate(resolve));
+			const updatedOpen = PullRequestOverviewPanel.createOrShow(telemetry, EXTENSION_URI, pullRequestManager, identity, updatedModel);
+			releaseInitialization!('main');
+			await Promise.all([initialOpen, updatedOpen]);
+
+			const panel = PullRequestOverviewPanel.findPanel(identity.owner, identity.repo, identity.number);
+			assert.strictEqual(panel?.getCurrentTitle(), '#1000 Updated title');
+			assert.strictEqual(panel?.getCurrentItem(), updatedModel);
+		});
+
+		it('does not post a stale timeline after a newer update', async function () {
+			const firstItem = convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(1000).title('Initial title').build(), repo);
+			const firstModel = new PullRequestModel(credentialStore, telemetry, repo, remote, firstItem);
+			const updatedItem = convertRESTPullRequestToRawPullRequest(new PullRequestBuilder().number(1000).title('Updated title').build(), repo);
+			const updatedModel = new PullRequestModel(credentialStore, telemetry, repo, remote, updatedItem);
+			const identity = { owner: firstModel.remote.owner, repo: firstModel.remote.repositoryName, number: firstModel.number };
+			const staleEvents: TimelineEvent[] = [];
+			let resolveTimeline: (events: TimelineEvent[]) => void;
+			const timelinePromise = new Promise<TimelineEvent[]>(resolve => resolveTimeline = resolve);
+			sinon.stub(firstModel, 'getReviewRequests').resolves([]);
+			sinon.stub(firstModel, 'getTimelineEvents').returns(timelinePromise);
+			sinon.stub(firstModel, 'validateDraftMode').resolves(false);
+			sinon.stub(firstModel, 'getStatusChecks').resolves([{ state: CheckState.Success, statuses: [] }, null]);
+			sinon.stub(updatedModel, 'getReviewRequests').resolves([]);
+			sinon.stub(updatedModel, 'getTimelineEvents').resolves([]);
+			sinon.stub(updatedModel, 'validateDraftMode').resolves(false);
+			sinon.stub(updatedModel, 'getStatusChecks').resolves([{ state: CheckState.Success, statuses: [] }, null]);
+
+			await PullRequestOverviewPanel.createOrShow(telemetry, EXTENSION_URI, pullRequestManager, identity, firstModel);
+			const panel = PullRequestOverviewPanel.findPanel(identity.owner, identity.repo, identity.number)!;
+			let releaseTimelineProcessing: () => void;
+			const timelineProcessingBlocked = new Promise<void>(resolve => releaseTimelineProcessing = resolve);
+			sinon.stub(panel as any, 'processTimelineEvents').callsFake(async (events: TimelineEvent[]) => {
+				if (events === staleEvents) {
+					await timelineProcessingBlocked;
+				}
+				return events;
+			});
+			const postMessage = sinon.spy(panel as any, '_postMessage');
+
+			resolveTimeline!(staleEvents);
+			await new Promise(resolve => setImmediate(resolve));
+			await PullRequestOverviewPanel.createOrShow(telemetry, EXTENSION_URI, pullRequestManager, identity, updatedModel);
+			releaseTimelineProcessing!();
+			await new Promise(resolve => setImmediate(resolve));
+
+			const postedStaleTimeline = postMessage.getCalls().some(call =>
+				call.args[0]?.command === 'pr.update' && call.args[0].pullrequest?.events === staleEvents
+			);
+			assert.strictEqual(postedStaleTimeline, false);
+		});
+
 		it('creates separate panels for different PRs', async function () {
 			const createWebviewPanel = sinon.spy(vscode.window, 'createWebviewPanel');
 

webviews/common/context.tsx

@@ -551,6 +551,8 @@ export class PRContext {
 				return;
 			case 'pr.initialize':
 				return this.setPR(message.pullrequest);
+			case 'pr.update':
+				return this.updatePR(message.pullrequest);
 			case 'update-state':
 				return this.updatePR({ state: message.state });
 			case 'pr.update-checkout-status':

webviews/editorWebview/test/overview.test.tsx

@@ -59,4 +59,21 @@ describe('Overview', function () {
 		assert(stickyHeader);
 		assert(!stickyHeader.classList.contains('visible'));
 	});
+
+	it('applies deferred pull request updates', function () {
+		const pr = new PullRequestBuilder().build();
+		const context = new PRContext(pr);
+
+		context.handleMessage({
+			command: 'pr.update',
+			pullrequest: {
+				events: [],
+				currentUserReviewState: 'APPROVED',
+			},
+		});
+
+		assert.deepStrictEqual(context.pr?.events, []);
+		assert.strictEqual(context.pr?.currentUserReviewState, 'APPROVED');
+		assert.strictEqual(context.pr?.title, pr.title);
+	});
 });