Skip to content

Commit

Permalink
[fix] transform links to be absolute to avoid console error when navi…
Browse files Browse the repository at this point in the history
…gating (#5583)

* make link hrefs absolute - fixes #3748

* ugh stfu eslint you miserable funsucker

* update test

* Create stupid-dolls-fly.md

* Update packages/kit/src/runtime/client/client.js

Co-authored-by: Ben McCann <[email protected]>

* Update packages/kit/test/apps/basics/test/test.js

Co-authored-by: Rich Harris <[email protected]>

* see if this makes different tests fail on safari

* Revert "see if this makes different tests fail on safari"

This reverts commit 91049b7.

* i hate safari with the fire of a thousand suns

* Update .changeset/stupid-dolls-fly.md

* Update packages/kit/src/runtime/client/client.js

* Update packages/kit/src/runtime/client/client.js

* ugh this is easier than fighting with typescript

Co-authored-by: Ben McCann <[email protected]>
  • Loading branch information
Rich-Harris and benmccann authored Jul 20, 2022
1 parent 400f415 commit 7943e14
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 4 deletions.
5 changes: 5 additions & 0 deletions .changeset/stupid-dolls-fly.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@sveltejs/kit": patch
---

[fix] transform link[rel=icon] to be absolute to avoid console error when navigating
7 changes: 7 additions & 0 deletions packages/kit/src/runtime/client/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -1220,6 +1220,13 @@ export function create_client({ target, session, base, trailing_slash }) {
}
});

// fix link[rel=icon], because browsers will occasionally try to load relative
// URLs after a pushState/replaceState, resulting in a 404 — see
// https://github.com/sveltejs/kit/issues/3748#issuecomment-1125980897
for (const link of document.querySelectorAll('link')) {
if (link.rel === 'icon') link.href = link.href;
}

addEventListener('pageshow', (event) => {
// If the user navigates to another site and then uses the back button and
// bfcache hits, we need to set navigating to null, the site doesn't know
Expand Down
22 changes: 18 additions & 4 deletions packages/kit/test/apps/basics/test/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1043,15 +1043,29 @@ test.describe('$app/paths', () => {
);
});

test('replaces %sveltekit.assets% in template with relative path', async ({ page }) => {
// some browsers will re-request assets after a `pushState`
// https://github.com/sveltejs/kit/issues/3748#issuecomment-1125980897
test('replaces %sveltekit.assets% in template with relative path, and makes it absolute in the client', async ({
baseURL,
page,
javaScriptEnabled
}) => {
const absolute = `${baseURL}/favicon.png`;

await page.goto('/');
expect(await page.getAttribute('link[rel=icon]', 'href')).toBe('./favicon.png');
expect(await page.getAttribute('link[rel=icon]', 'href')).toBe(
javaScriptEnabled ? absolute : './favicon.png'
);

await page.goto('/routing');
expect(await page.getAttribute('link[rel=icon]', 'href')).toBe('./favicon.png');
expect(await page.getAttribute('link[rel=icon]', 'href')).toBe(
javaScriptEnabled ? absolute : './favicon.png'
);

await page.goto('/routing/rest/foo/bar/baz');
expect(await page.getAttribute('link[rel=icon]', 'href')).toBe('../../../../favicon.png');
expect(await page.getAttribute('link[rel=icon]', 'href')).toBe(
javaScriptEnabled ? absolute : '../../../../favicon.png'
);
});
});

Expand Down

0 comments on commit 7943e14

Please sign in to comment.