fix(site): restore SPA path before router boots so 404 page renders#15
Merged
Conversation
Move the GitHub Pages deep-link restoration step from main.tsx into an inline <script> in index.html's <head>. Module scripts are deferred, so the old restore ran after createBrowserRouter had already captured the post-bounce '/?p=...' URL and matched it to the home route. The inline script runs synchronously before the bundle, so the router boots with the original path and the catch-all NotFoundPage renders. Also polish NotFoundPage: show the attempted path, add Raw JSON and Source links, larger visual hierarchy that matches the rest of the site.
There was a problem hiding this comment.
Pull request overview
Fixes GitHub Pages deep-link handling for the report SPA by restoring the original path before React Router initializes, ensuring unknown routes correctly render NotFoundPage instead of silently landing on the home page.
Changes:
- Moved the
?p=...deep-link restoration logic fromsite/src/main.tsxto an inline<script>insite/index.html’s<head>. - Updated
public/404.htmldocumentation comment to reference the new restore location. - Refreshed
NotFoundPageUI and added links to Raw JSON and source.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| site/src/pages/NotFoundPage.tsx | Improves 404 UX, shows attempted path, adds navigation and resource links. |
| site/src/main.tsx | Removes late path-restore logic and leaves a note pointing to index.html. |
| site/public/404.html | Updates fallback comment to reference the new restore step location. |
| site/index.html | Adds an inline head script to restore deep-linked paths before the router boots. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Build the restored URL by feeding `p` into the URL constructor and appending any extra search params from the bounce URL. The old version concatenated strings, which produced a malformed `/foo?a=1?x=2` if `p` already carried a query (or an extra param landed on the bounce URL). Also reject `p` values that don't look like same-origin relative paths so a crafted `?p=//host/foo` can't try to flip the visible origin. Addresses Copilot feedback on #15.
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.
Changes
site/src/main.tsxinto an inline<script>insite/index.html's<head>.main.tsxto a short note pointing at the new location.NotFoundPage: show the attempted path in mono, add Raw JSON and Source links, scale up the visual hierarchy to match the other pages.public/404.htmlto referenceindex.htmlinstead ofmain.tsx.Why this was needed
Going to
scanner.registry.coder.com/anything-that-isn't-a-routewas silently landing on the home page instead ofNotFoundPage. The catch-all route already existed; it just never got hit.The flow on GitHub Pages is:
public/404.html(HTTP 404).404.htmlredirects to/?p=%2Fanything-that-isn't-a-routeso the SPA gets a chance to render.index.html, which has<script type="module" src="/src/main.tsx">.main.tsximports./App→./router, which callscreateBrowserRouter(...)at module load. At that momentwindow.location.pathnameis still/, so the router locks in the home route.history.replaceStateupdated the URL bar to/anything-that-isn't-a-routebut the router had already committed to/.Moving the restoration into a synchronous inline script in
<head>makes it run before the deferred module bundle, so the router sees the original path and the*catch-all rendersNotFoundPageas intended.This PR was prepared with help from Coder Agents.