feat/redirects: Apply heading (#fragment) redirects in the browser - #1879
Draft
marcleblanc2 wants to merge 1 commit into
Draft
feat/redirects: Apply heading (#fragment) redirects in the browser#1879marcleblanc2 wants to merge 1 commit into
marcleblanc2 wants to merge 1 commit into
Conversation
Browsers never send the URL fragment to the server, so the 386 entries in src/data/redirects.ts whose source contains '#' (e.g. '/cody/quickstart#introduction') have never matched in src/middleware.ts. Anyone following an old anchored link lands on the not-found page or the unredirected page. Add a FragmentRedirect client component, rendered once in the root layout, that reads location.hash after load (and on hashchange), looks up pathname + hash in the same redirects list, and rewrites the URL: a same-page hit becomes history.replaceState plus a scroll to the new heading, a cross-page hit becomes location.replace. It also handles the case where the middleware already redirected the path and the browser carried a stale fragment along. Export redirects.ts as ESM so both the middleware and the client component import the same list; no data is moved or split. Co-authored-by: Amp <amp@ampcode.com> Amp-Thread-ID: https://ampcode.com/threads/T-01a07e6f-db41-74af-bbeb-f8952e637289
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
src/data/redirects.tscontains 386 entries whosesourceincludes a#fragment, e.g.Browsers never send the fragment to the server, so
src/middleware.ts(which matchesr.source === pathname) has never fired for any of them — and neither did the Nextredirects()config they were originally added to in 2023. Verified on the live site: https://sourcegraph.com/docs/batch_changes/explanations/permissions_in_batch_changes#code-host-interactions-in-batch-changes serves the not-found page with noLocationheader.This also means there has been no way to add a redirect when renaming a heading, so inbound anchored links silently break.
Change
src/components/FragmentRedirect.tsx: client component rendered once in the root layout. After load (and onhashchange) it looks uppathname + location.hashin the#entries of the same redirects list. Same-page hit →history.replaceState+ scroll to the new heading (no reload). Cross-page hit →location.replace. It also handles the two-step case where the middleware redirects the path and the browser carries a stale fragment onto the new page.src/data/redirects.ts: exported as ESM (export const updatedRedirectsData) so the middleware and the client component import the same list. No entries were moved, added, or removed. Added a header comment explaining which entries are handled where.src/middleware.ts:require→import. Behaviour unchanged.Heading renames are now handled by adding one line to the existing list:
{source: '/page#old-heading', destination: '/page#new-heading'}.Verification (dev server, headless Chromium)
/batch_changes/explanations/permissions_in_batch_changes#code-host-interactions-in-batch-changes(404 today)/batch-changes/permissions-in-batch-changes#code-host-interactions-in-batch-changes, scrolled to heading/cody/quickstart#introduction(same-page rename)/cody/quickstart#cody-quickstart, no reload, scrolled to heading/cody/clients/install-vscode#requirements(what the browser lands on after the middleware's 307 from/cody/overview/install-vscode#requirements)/cody/clients/install-vscode#prerequisites/cody/quickstart#prerequisites(valid anchor, control)/admin/http_https_configuration(middleware path redirect, control)/self-hosted/http-https-configurationeslintclean on changed files;node dev/check-links.mjspasses;tscerrors are pre-existing (contentlayer/generated).Notes / follow-ups (not in this PR)
#entries now ship to the client as part of the redirects chunk (~15 KB compressed, cached across pages). Most of the 386 point at long-gone Cody pages and could be pruned./batch_changes/explanations/permissions_in_batch_changes(no anchor) still 404s; it needs a fragment-less entry.createRedirectUrlhardcodes/docs, so middleware redirects 404 in local dev;notFound()pages return HTTP 200 in production.Amp threads