Skip to content

Conversation

@teemingc
Copy link
Member

@teemingc teemingc commented Jan 9, 2025

fixes #13287

This PR fixes the client-side external link evaluation so that when using the hash router, accessing /index.html is considered an internal link. This prevents the page from reloading infinitely because the router incorrectly considered /index.html an external link.

There's also a slight doc change. Instead of suggesting all links start with /#/, suggesting links start with # allows the router to preserve the /index.html pathname. For example, links to /about should be #/about instead of /#/about so that it navigates to /index.html#/about.


Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

Edits

  • Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed.

@changeset-bot
Copy link

changeset-bot bot commented Jan 9, 2025

🦋 Changeset detected

Latest commit: 2abef82

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@sveltejs/kit Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@teemingc teemingc changed the title fix and changeset fix: treat /index.html as an internal link when hash router is used Jan 9, 2025
@Rich-Harris
Copy link
Member

preview: https://svelte-dev-git-preview-kit-13296-svelte.vercel.app/

this is an automated message

@teemingc teemingc changed the title fix: treat /index.html as an internal link when hash router is used fix: treat /index.html as an internal link when the hash router is used Jan 9, 2025
@teemingc teemingc marked this pull request as draft January 9, 2025 11:48
@teemingc teemingc changed the title fix: treat /index.html as an internal link when the hash router is used fix: correctly detect internal links when hash router is used Jan 9, 2025
@teemingc
Copy link
Member Author

teemingc commented Jan 9, 2025

Seems to still infinitely reload if we embed the app in an iframe using srcdoc because the URL becomes about:srcdoc and the pathname is srcdoc.

EDIT: I think we'll just deal with the embedded app html text case in a different PR as there are more compatibility issues to deal with first #13226 . Embedding through a URL src seems to work fine.

@teemingc teemingc marked this pull request as ready for review January 10, 2025 07:11
@sateeshsai
Copy link

sateeshsai commented Jan 10, 2025

There's also a slight doc change. Instead of suggesting all links start with /#/, suggesting links start with # allows the router to preserve the /index.html pathname. For example, links to /about should be #/about instead of /#/about so that it navigates to /index.html#about.

This is great. I was going to raise an issue for this. '.../index.html/#/about' causes browser navigation in pages served by certain web servers.

Would this change work for urls that have the page name other than /index.html? For example, .../somepage.html

@stephenlrandall

This comment was marked as off-topic.

@teemingc
Copy link
Member Author

Would this change work for urls that have the page name other than /index.html? For example, .../somepage.html

No, because the hash router build currently always generates an index.html page as the root page.

Wouldn't /index.html#about be expected to land on an element with id about on the /index.html page?

Yes, but you don't want to write all your links as href="/index.html#about" so you can just write href="#about" for the same effect.

@stephenlrandall
Copy link
Contributor

No, because the hash router build currently always generates an index.html page as the root page.

This actually isn't documented at the moment, and it's not clear from the documentation how this interacts with the adapter-static fallback page option.

Wouldn't /index.html#about be expected to land on an element with id about on the /index.html page?

Yes, but you don't want to write all your links as href="/index.html#about" so you can just write href="#about" for the same effect.

So in the event someone has an "About this project" section on their home page with id="about" but also a page called "About" at /about, how would you link to each? Should href="#about" take you down to the section or over to the other page? Would something like href="#/about#authors" correctly take you to the "Authors" section over on /about? I apologize for all the questions, but it's not clear from the documentation how this is supposed to work.

@teemingc
Copy link
Member Author

Wouldn't /index.html#about be expected to land on an element with id about on the /index.html page?

Yes, but you don't want to write all your links as href="/index.html#about" so you can just write href="#about" for the same effect.

So in the event someone has an "About this project" section on their home page with id="about" but also a page called "About" at /about, how would you link to each? Should href="#about" take you down to the section or over to the other page?

Sorry, I'm mistaken. The link should be href="#/about". The slash after the hash will cause it to navigate to a different page.

Would something like href="#/about#authors" correctly take you to the "Authors" section over on /about? I apologize for all the questions, but it's not clear from the documentation how this is supposed to work.

Yes, that will take you to authors section in the about page. Perhaps you can create a new issue and outline the hash router mechanics that require clarification in the documentation.

@dummdidumm dummdidumm merged commit 1c77e28 into main Jan 15, 2025
14 checks passed
@dummdidumm dummdidumm deleted the fix-hash-router-index.html branch January 15, 2025 16:46
@github-actions github-actions bot mentioned this pull request Jan 15, 2025
@sateeshsai
Copy link

sateeshsai commented Aug 13, 2025

Would this change work for urls that have the page name other than /index.html? For example, .../somepage.html

No, because the hash router build currently always generates an index.html page as the root page.

What is the recommended workaround if my html file isn't named index.html. E.g., test.html

@teemingc
Copy link
Member Author

Would this change work for urls that have the page name other than /index.html? For example, .../somepage.html

No, because the hash router build currently always generates an index.html page as the root page.

What is the recommended workaround if my html file isn't named index.html. E.g., test.html

It should be easy enough to rename it in a post-build script. We should continue further discussion in the discussion boards or on discord

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Infinite page reloads when a index.html (built with adapter-static) is opened as explicit url (www.domain.com/index.html)

6 participants