A file's blame no longer holds up its page. The blob page waited for repos' blame before sending anything, and a file's first blame at a commit walks its history: 1.2 to 7.1 seconds since b3f9aeda (Workers Logs answered_ms, after the deploy), 8.5 seconds for the whole page on apps/api/Cargo.toml. The page now opens with the file as it is and "Working out who changed each line" above it, and each line's commit fills in when the blame arrives (streamed from the loader; useArrived, now in lib/use-arrived.ts for the pull request page too). One that cannot be worked out says so and leaves the file. A blame seen before still comes from repos' cache at once. The blame guide says so. Checked: the site's typecheck (tsc -b --force) and unit tests (950 passing).
- G1t-Session:
- ses_01m4y12gdde2r9q832m7xk87kd
5 files+63−460/5 viewed
| 96 | 96 | ||
| 97 | 97 | Blame reads up to 400 commits back, for up to six seconds. A line older | |
| 98 | 98 | than that is credited to the oldest commit read that has it, and the view | |
| 99 | − | says the history is long. A file's blame at a commit is kept once worked | |
| 99 | + | says the history is long. While it is worked out the file shows as it is, | |
| 100 | + | with "Working out who changed each line" above it, and each line's commit | |
| 101 | + | fills in when it is ready. A file's blame at a commit is kept once worked | |
| 100 | 102 | out, so opening it again, or from a commit's link, shows at once. Search | |
| 101 | 103 | engines and other crawlers are asked to leave blame pages alone | |
| 102 | 104 | (`Disallow: /*?blame=` in `/robots.txt`); the file itself stays open to |
| 1 | − | import { BookOpen, Check, ChevronDown, Code2, CornerLeftUp, File, FileArchive, Folder, FolderGit2, GitBranch, History, Search, SquareTerminal, Tag as TagIcon } from "lucide-react"; | |
| 1 | + | import { BookOpen, Check, ChevronDown, Code2, CornerLeftUp, File, FileArchive, Folder, FolderGit2, GitBranch, History, LoaderCircle, Search, SquareTerminal, Tag as TagIcon } from "lucide-react"; | |
| 2 | 2 | import { type ReactNode, Suspense } from "react"; | |
| 3 | 3 | import { Await, Form, Link } from "react-router"; | |
| 4 | 4 | ||
| ⋯ | |||
| 449 | 449 | blob: Blob; | |
| 450 | 450 | /** Syntax-highlighted HTML per line, when the language is known. */ | |
| 451 | 451 | html: string[] | null; | |
| 452 | − | /** Shown instead of the plain file when asked for, with HTML per line. */ | |
| 453 | − | blame?: { blame: ShownBlame; lines: string[] | null } | null; | |
| 452 | + | /** Shown instead of the plain file when asked for, with HTML per line; | |
| 453 | + | * "waiting" while it streams in, "failed" when it could not be had (the | |
| 454 | + | * plain file shows meanwhile). */ | |
| 455 | + | blame?: { blame: ShownBlame; lines: string[] | null } | "waiting" | "failed" | null; | |
| 454 | 456 | }) { | |
| 455 | 457 | const { repo, ref, path, size, text } = blob; | |
| 456 | 458 | const lines = text?.replace(/\n$/, "").split("\n"); | |
| ⋯ | |||
| 484 | 486 | <Card tone="plain" className="overflow-hidden"> | |
| 485 | 487 | <div className="flex items-center gap-3 border-b border-line bg-surface px-3 py-2 text-xs text-muted sm:px-4 sm:py-2.5"> | |
| 486 | 488 | {lines && <span>{lines.length.toLocaleString("en-US")} lines</span>} | |
| 489 | + | {blame === "waiting" && ( | |
| 490 | + | <span className="flex items-center gap-1.5" role="status"> | |
| 491 | + | <LoaderCircle size={12} className="animate-spin" aria-hidden /> | |
| 492 | + | Working out who changed each line | |
| 493 | + | </span> | |
| 494 | + | )} | |
| 495 | + | {blame === "failed" && <span role="status">Blame could not be worked out; try again in a moment</span>} | |
| 487 | 496 | <span className="hidden min-[400px]:inline">{size.toLocaleString("en-US")} bytes</span> | |
| 488 | 497 | <span className="ml-auto flex items-center gap-2"> | |
| 489 | 498 | {lines && ( | |
| ⋯ | |||
| 497 | 506 | </a> | |
| 498 | 507 | </span> | |
| 499 | 508 | </div> | |
| 500 | − | {blame && lines ? ( | |
| 509 | + | {typeof blame === "object" && blame && lines ? ( | |
| 501 | 510 | <BlameView base={base} path={path} lines={lines} html={blame.lines} blame={blame.blame} /> | |
| 502 | 511 | ) : lines ? ( | |
| 503 | 512 | <CodeLines lines={lines} html={html} marked={marked} /> | |
| 1 | + | import { useEffect, useState } from "react"; | |
| 2 | + | ||
| 3 | + | /** | |
| 4 | + | * `initial` until `promise` settles, then what it resolved to: for a value | |
| 5 | + | * a loader streams in after the page, shown in place when it arrives. A | |
| 6 | + | * promise that rejects leaves `initial`. | |
| 7 | + | */ | |
| 8 | + | export function useArrived<T, I = T>(promise: Promise<T>, initial: I): T | I { | |
| 9 | + | const [arrived, setArrived] = useState<{ from: Promise<T>; value: T } | null>(null); | |
| 10 | + | useEffect(() => { | |
| 11 | + | let current = true; | |
| 12 | + | promise.then( | |
| 13 | + | (value) => current && setArrived({ from: promise, value }), | |
| 14 | + | () => {}, | |
| 15 | + | ); | |
| 16 | + | return () => { | |
| 17 | + | current = false; | |
| 18 | + | }; | |
| 19 | + | }, [promise]); | |
| 20 | + | return arrived?.from === promise ? arrived.value : initial; | |
| 21 | + | } |
| 4 | 4 | import { page } from "../../lib/meta"; | |
| 5 | 5 | import { CodeownersFileErrors } from "../../components/codeowners"; | |
| 6 | 6 | import { BlobView } from "../../components/repo-view"; | |
| 7 | + | import type { ShownBlame } from "../../lib/commit-people"; | |
| 8 | + | import { useArrived } from "../../lib/use-arrived"; | |
| 7 | 9 | import { highlightLines } from "../../lib/highlight.server"; | |
| 8 | 10 | import { repos, work } from "../../lib/services.server"; | |
| 9 | 11 | import { showCommits } from "../../lib/commit-people.server"; | |
| ⋯ | |||
| 22 | 24 | const path = { namespace: params.owner, name: params.repo }; | |
| 23 | 25 | const file = params["*"] ?? ""; | |
| 24 | 26 | const wantsBlame = new URL(request.url).searchParams.has("blame"); | |
| 25 | − | const [found, blame, list, checked] = await Promise.all([ | |
| 27 | + | // A first blame can take seconds (repos walks the file's history), so it | |
| 28 | + | // streams in after the file; a later one comes from repos' cache at once. | |
| 29 | + | const blameFound = wantsBlame ? repos.blame(path, viewer, params.ref, file) : null; | |
| 30 | + | blameFound?.catch(() => null); | |
| 31 | + | const [found, list, checked] = await Promise.all([ | |
| 26 | 32 | repos.blob(path, viewer, params.ref, file), | |
| 27 | − | wantsBlame ? repos.blame(path, viewer, params.ref, file) : null, | |
| 28 | 33 | // For the branch menu; the page still shows without it. | |
| 29 | 34 | repos.branches(path, viewer).catch(() => null), | |
| 30 | 35 | // A CODEOWNERS file is checked as it is shown; the file shows without it. | |
| ⋯ | |||
| 35 | 40 | // A branch that was renamed: the same file on its new name. | |
| 36 | 41 | if (!found.ok && found.error.code === "not_found") await redirectIfBranchRenamed(request, path, viewer, params.ref); | |
| 37 | 42 | const blob = unwrap(found); | |
| 38 | − | if (blame?.ok && blob.text != null) { | |
| 39 | − | return { | |
| 40 | − | blob, | |
| 41 | − | html: null, | |
| 42 | − | blame: { | |
| 43 | − | // Every commit's people at once. | |
| 44 | − | blame: { ...blame.value, commits: await showCommits(blame.value.commits) }, | |
| 45 | − | lines: await highlightLines(blob.path, blob.text), | |
| 46 | − | }, | |
| 47 | − | branches, | |
| 48 | − | codeowners, | |
| 49 | − | }; | |
| 50 | − | } | |
| 51 | − | return { | |
| 52 | − | blob, | |
| 53 | − | html: blob.text == null ? null : await highlightLines(blob.path, blob.text), | |
| 54 | − | blame: null, | |
| 55 | − | branches, | |
| 56 | − | codeowners, | |
| 57 | − | }; | |
| 43 | + | const html = blob.text == null ? null : await highlightLines(blob.path, blob.text); | |
| 44 | + | // Each line's commit, with every commit's people at once; null when it | |
| 45 | + | // could not be worked out, and the file shows as it is. | |
| 46 | + | const blame: Promise<ShownBlame | null> | null = | |
| 47 | + | blameFound && blob.text != null | |
| 48 | + | ? blameFound | |
| 49 | + | .then(async (answered) => (answered.ok ? { ...answered.value, commits: await showCommits(answered.value.commits) } : null)) | |
| 50 | + | .catch(() => null) | |
| 51 | + | : null; | |
| 52 | + | return { blob, html, blame, branches, codeowners }; | |
| 58 | 53 | } | |
| 59 | 54 | ||
| 60 | 55 | export default function Blob({ loaderData, params }: Route.ComponentProps) { | |
| 61 | − | const { codeowners, blob } = loaderData; | |
| 56 | + | const { codeowners, blob, html } = loaderData; | |
| 62 | 57 | // Its errors' lines are marked, when they are this file's. | |
| 63 | 58 | const own = codeowners != null && codeowners.path === blob.path; | |
| 59 | + | // undefined while the blame is on its way, null when it could not be had. | |
| 60 | + | const blame = useArrived(loaderData.blame ?? NO_BLAME, undefined); | |
| 64 | 61 | return ( | |
| 65 | 62 | <BlobView | |
| 66 | 63 | blob={blob} | |
| 67 | − | html={loaderData.html} | |
| 68 | − | blame={loaderData.blame} | |
| 64 | + | html={html} | |
| 65 | + | blame={loaderData.blame == null ? null : blame === undefined ? "waiting" : blame === null ? "failed" : { blame, lines: html }} | |
| 69 | 66 | branches={loaderData.branches} | |
| 70 | 67 | notice={ | |
| 71 | 68 | codeowners && ( | |
| ⋯ | |||
| 76 | 73 | /> | |
| 77 | 74 | ); | |
| 78 | 75 | } | |
| 76 | + | ||
| 77 | + | const NO_BLAME: Promise<null> = Promise.resolve(null); | |
| 22 | 22 | Users, | |
| 23 | 23 | Wrench, | |
| 24 | 24 | } from "lucide-react"; | |
| 25 | − | import { Suspense, useEffect, useState } from "react"; | |
| 25 | + | import { Suspense, useState } from "react"; | |
| 26 | 26 | import { Await, Form, Link, redirect } from "react-router"; | |
| 27 | 27 | ||
| 28 | 28 | import { | |
| ⋯ | |||
| 48 | 48 | import { highlightFirstFiles } from "../../lib/highlight.server"; | |
| 49 | 49 | import { excerpt, page } from "../../lib/meta"; | |
| 50 | 50 | import { openedBy } from "../../lib/opened-by"; | |
| 51 | + | import { useArrived } from "../../lib/use-arrived"; | |
| 51 | 52 | import { cloneUrl, useAddresses } from "../../lib/addresses"; | |
| 52 | 53 | import { DiffView } from "../../components/diff-view"; | |
| 53 | 54 | import { DetailsDisclosure, detailsSummary } from "../../components/details-disclosure"; | |
| ⋯ | |||
| 685 | 686 | } | |
| 686 | 687 | ||
| 687 | 688 | /** The box at the foot of the conversation saying what stands before a merge. */ | |
| 688 | − | /** `initial` until `promise` settles, then what it resolved to. */ | |
| 689 | − | function useArrived<T>(promise: Promise<T>, initial: T): T { | |
| 690 | − | const [arrived, setArrived] = useState<{ from: Promise<T>; value: T } | null>(null); | |
| 691 | − | useEffect(() => { | |
| 692 | − | let current = true; | |
| 693 | − | promise.then( | |
| 694 | − | (value) => current && setArrived({ from: promise, value }), | |
| 695 | − | () => {}, | |
| 696 | − | ); | |
| 697 | − | return () => { | |
| 698 | − | current = false; | |
| 699 | − | }; | |
| 700 | − | }, [promise]); | |
| 701 | − | return arrived?.from === promise ? arrived.value : initial; | |
| 702 | − | } | |
| 703 | 689 | ||
| 704 | 690 | function StatusBox({ children }: { children: React.ReactNode }) { | |
| 705 | 691 | return ( | |
Comments
Sign in to comment.