fix: seed the graph from the slug stamped on <body>, not the URL - #15
Open
thoreplass wants to merge 2 commits into
Open
fix: seed the graph from the slug stamped on <body>, not the URL#15thoreplass wants to merge 2 commits into
thoreplass wants to merge 2 commits into
Conversation
renderLocal() and showGlobalGraph() derived the current node from
location.pathname. The URL is not authoritative for this:
- a host may normalise the case of pretty URLs. Netlify lowercases
them, so a note published at Notes/Prices is served at /notes/prices
while the content index is still keyed by Notes/Prices.
- location.pathname is always percent-encoded, so a slug with
non-ASCII characters never matches its own key either.
In both cases the breadth-first search was seeded with a node that is not
in the graph, so it returned only itself and the panel rendered a single
unconnected dot. Every page on such a site is affected, including the
landing page.
Quartz already stamps the canonical slug on <body data-slug> and hands it
to plugins in the nav event, so read it from there and keep the URL as a
fallback for pages that carry no data-slug. getCurrentSlug() moves to
src/util/slug.ts so the resolution can be unit-tested on its own.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An attribute that is present but empty must be treated as missing, not returned as the current slug. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
The bug
Graph View renders a single unconnected dot on every page of a site whose slugs contain an uppercase or non-ASCII character. The panel mounts, the canvas is created, and
contentIndex.jsonloads correctly - the search is simply seeded with a node that is not in the graph.renderLocal()andshowGlobalGraph()derived the current node fromlocation.pathname, and the URL is not authoritative for this:Notes/Pricesis served at/notes/priceswhile the content index is still keyed byNotes/Prices.location.pathnameis always percent-encoded. A slug containing non-ASCII characters never matches its own key either, even when the case is right.The breadth-first search then starts from a slug that has no edges, returns only itself, and the panel draws one dot. Measured on a live Quartz 5 site (385 notes, 1304 links) served from Netlify:
contentIndex.jsondocument.body.dataset.slugSider/Priserlocation.pathname-> derived slugsider/priser383 of the 385 slugs on that site have an uppercase character, so effectively every page was affected, the landing page included.
The fix
Quartz already stamps the canonical slug on
<body data-slug>, and it is the same value the router hands plugins in thenavevent. Read it from there, and keep the URL as a fallback for pages that carry nodata-slug.getCurrentSlug()moves intosrc/util/slug.tsso the resolution can be unit-tested on its own -graph.inline.tsis@ts-nocheck'd and only reachable as a bundled string, so the previousgetSlugFromUrl()had no way to be covered.tsup's inline-script loader bundles withresolveDirset to the script's directory, so the relative import is inlined into the emitted script exactly as the@quartz-community/utilsimports already are; verified indist/.Tests
test/slug.test.tscovers four cases, and the first two fail onmain:<body>when the URL case differs<body>carries no slugnpm run typecheck,npm run lintandvitest run(8 tests) all pass, andnpm run buildsucceeds.🤖 Generated with Claude Code