fix: redirect hashes with different casing #586
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| for (const el of elements) { | ||
| if (el.id.toLocaleLowerCase() === hash.toLocaleLowerCase()) { | ||
| const url = $page.url; | ||
| url.hash = el.id; |
There was a problem hiding this comment.
is it necessary to mutate it? wouldn't you be able to construct the new url and return it without touching the store?
There was a problem hiding this comment.
yeah, mostly I wanted to know if I should even be able to. It doesn't seem like I should. I filed a feature request for that: sveltejs/kit#12870. And have updated this to make a copy
| const elements = document.querySelectorAll('*[id]'); | ||
| // if there's an exact match, use that. no need to redirect | ||
| // also handles the case where one appears twice with difference casing | ||
| // e.g. https://svelte.dev/docs/kit/@sveltejs-kit#redirect vs https://svelte.dev/docs/kit/@sveltejs-kit#Redirect | ||
| for (const el of elements) { |
There was a problem hiding this comment.
why are we iterating? just use a case-insensitive selector
const heading = document.querySelector(`[id="${id}" i]`);There was a problem hiding this comment.
updated to use that, but we still need to iterate. e.g. we have https://svelte.dev/docs/kit/@sveltejs-kit#redirect and https://svelte.dev/docs/kit/@sveltejs-kit#Redirect
There was a problem hiding this comment.
could do this!
document.querySelector(`[id="${id}"]`) ?? document.querySelector(`[id="${id}" i]`);There was a problem hiding this comment.
well it turns out that this issue isn't fixable. we need to change one of these ids so that they don't collide: #590
|
So here's the thing: this won't close #556, unless you also add a I think we should just fix the links. |
I fixed the links in our own docs already. What I'm worried about is breaking every link on the internet, which is what I'm trying to prevent here |
Co-authored-by: Rich Harris <[email protected]>
Co-authored-by: Rich Harris <[email protected]>
Co-authored-by: Rich Harris <[email protected]>
Co-authored-by: Rich Harris <[email protected]>
3 participants
Closes #556. This works pretty well.
Is it okay that I mutated the URL from the store on line 40?
And is using goto on line 49 the appropriate way to do this?