Repository navigation
fix(handbook): stop calling resolve('') in root-landing check - #172
Merged
Merged
Conversation
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.
+layout.sveltecomputedisRootLandingwith a second clausepage.url.pathname === resolveHref('').resolveHrefforwards to$app/pathsresolve(), which throws on any argument not starting with/. The validation is present in @sveltejs/kit 2.65.0 and 2.70.2; the earlier pinned 2.55.0 had none (resolve('')returnedbase + '/'). The lockfile first picked up a validating kit in b677828 (#149).Consequences, reproduced locally with
BASE_PATH=/beacon npm run buildon kit 2.70.2:[500] GET /beacon/latest ... Cannot use resolve(...)in the build log).prerender.handleHttpError: 'warn'inhandbook/svelte.config.jsdowngraded those to warnings, so CI was green while the Pages artifact contained onlyindex.html,404.html, the redirect stubs and the.md/llms.txtendpoints (4 HTML files total). The currently published site, from the Sep 11 workflow_dispatch run, is in that state:/beacon/latest/and/beacon/v2000.0.0/introductionreturn 404.404.htmlwith a 404 status for any missing route; the client app boots on it and the sameresolve('')throws uncaught during render, leaving a blank page withCannot use resolve(...) (got "")in the console.The clause adds nothing even where it does not throw: in the client and during prerendering,
resolve('/')already equalsbase + '/', the only value the second clause could ever match. This removes it.The PR also contains:
DocLayoutderived its version prefix by strippingbasefrompage.url.pathname. During prerendering$app/pathsbaseis relative (overridden per request), so the prefix came out empty andVersionLinkemitted root-level hrefs —../lensfrom/latest/select,../migrationfrom/v1000.0.0/introduction— the two 404s the crawl warned about. It now derives frompage.route.id, which carries no base on server or client; the prev/next index comparison had the same mismatch and is fixed the same way.](/latest/hooks-overview)and siblings) lackedbaseentirely: they 404 outside/beaconfor users and fail the crawl's base check. They now go throughVersionLink.prerender.handleHttpError: 'warn'is removed, restoring the kit default'fail': any non-OK response during prerendering, including 500s from render crashes, fails the build instead of shipping a hollow artifact green.Verification, same build command, after all changes: exit 0, zero 404/500 lines during prerendering, 232 HTML files including
latest.htmlandv2000.0.0/introduction.html; static-HTML anchors resolve inside base (e.g.../latest/hooks-overviewfrom/beacon/latest/state).