Skip to content

[diffshub] rewrite PR-scoped commit URLs to /commit/{sha}#703

Merged
necolas merged 1 commit into
beta-1.2from
nicolas/diffshub-route-handling
May 19, 2026
Merged

[diffshub] rewrite PR-scoped commit URLs to /commit/{sha}#703
necolas merged 1 commit into
beta-1.2from
nicolas/diffshub-route-handling

Conversation

@necolas
Copy link
Copy Markdown
Contributor

@necolas necolas commented May 19, 2026

Map /owner/repo/pull/N/(changes|files)/{sha} to /owner/repo/commit/{sha} so a GitHub PR URL scoped to a specific commit resolves to the standalone commit view, which is the only form the diff loader can fetch.

The rewrite runs in two places:

  • normalizeGitHubPath covers form input via getPatchViewerHref.
  • The catch-all viewer route now redirects to the canonical path when someone navigates directly, fixing the "couldn't load the diff" failure on pasted URLs.

Extracts resolveDiffshubViewerRoute from the route page so the redirect/render decision is unit-testable.

Example of successful mapping (and redirect):

https://pierre-docs-diffshub-git-nicolas-4fb69f-pierre-computer-company.vercel.app/pierrecomputer/pierre/pull/692/changes/3bbb3d55da7bbc1b290e1c4c0c5430016ec4d0cf

image

Map `/owner/repo/pull/N/(changes|files)/{sha}` to `/owner/repo/commit/{sha}`
so a GitHub PR URL scoped to a specific commit resolves to the standalone
commit view, which is the only form the diff loader can fetch.

The rewrite runs in two places:
- `normalizeGitHubPath` covers form input via `getPatchViewerHref`.
- The catch-all viewer route now redirects to the canonical path when
  someone navigates directly, fixing the "couldn't load the diff" failure
  on pasted URLs.

Extracts `resolveDiffshubViewerRoute` from the route page so the
redirect/render decision is unit-testable.
@necolas necolas requested a review from amadeus May 19, 2026 17:39
@vercel
Copy link
Copy Markdown

vercel Bot commented May 19, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
pierre-docs-diffshub Ready Ready Preview May 19, 2026 5:39pm
pierre-docs-trees Ready Ready Preview May 19, 2026 5:39pm
pierrejs-diff-demo Ready Ready Preview May 19, 2026 5:39pm
pierrejs-docs Ready Ready Preview May 19, 2026 5:39pm

Request Review

@necolas necolas requested a review from mdo May 19, 2026 19:05
Copy link
Copy Markdown
Member

@amadeus amadeus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙏

@necolas necolas merged commit ebf89d5 into beta-1.2 May 19, 2026
12 checks passed
@necolas necolas deleted the nicolas/diffshub-route-handling branch May 19, 2026 19:11
amadeus pushed a commit that referenced this pull request May 20, 2026
Map `/owner/repo/pull/N/(changes|files)/{sha}` to `/owner/repo/commit/{sha}`
so a GitHub PR URL scoped to a specific commit resolves to the standalone
commit view, which is the only form the diff loader can fetch.

The rewrite runs in two places:
- `normalizeGitHubPath` covers form input via `getPatchViewerHref`.
- The catch-all viewer route now redirects to the canonical path when
  someone navigates directly, fixing the "couldn't load the diff" failure
  on pasted URLs.

Extracts `resolveDiffshubViewerRoute` from the route page so the
redirect/render decision is unit-testable.
amadeus pushed a commit that referenced this pull request May 20, 2026
Map `/owner/repo/pull/N/(changes|files)/{sha}` to `/owner/repo/commit/{sha}`
so a GitHub PR URL scoped to a specific commit resolves to the standalone
commit view, which is the only form the diff loader can fetch.

The rewrite runs in two places:
- `normalizeGitHubPath` covers form input via `getPatchViewerHref`.
- The catch-all viewer route now redirects to the canonical path when
  someone navigates directly, fixing the "couldn't load the diff" failure
  on pasted URLs.

Extracts `resolveDiffshubViewerRoute` from the route page so the
redirect/render decision is unit-testable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants