From 1e7decd0e3e3e15d1da8a19ff07d854d74f5003d Mon Sep 17 00:00:00 2001 From: Mihael Date: Sat, 12 Feb 2022 20:44:04 +0100 Subject: [PATCH 01/63] push --- .changeset/shy-taxis-drum.md | 5 +++ packages/kit/src/runtime/client/router.js | 42 +++++++++++------------ 2 files changed, 25 insertions(+), 22 deletions(-) create mode 100644 .changeset/shy-taxis-drum.md diff --git a/.changeset/shy-taxis-drum.md b/.changeset/shy-taxis-drum.md new file mode 100644 index 000000000000..4867115e5ca0 --- /dev/null +++ b/.changeset/shy-taxis-drum.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +fixes scroll navigation issue and removes the need for scroll event listener diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index 948519835afb..69e7a14e389a 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -72,6 +72,20 @@ export class Router { }; } + save_scroll_state() { + const new_state = { + ...(history.state || {}), + 'sveltekit:scroll': scroll_state() + }; + + // need this in case it was set in previous state + if (new_state['sveltekit:scroll'].x === 0 && new_state['sveltekit:scroll'].y === 0) { + delete new_state['sveltekit:scroll']; + } + + history.replaceState(new_state, document.title, window.location.href); + } + init_listeners() { if ('scrollRestoration' in history) { history.scrollRestoration = 'manual'; @@ -96,6 +110,8 @@ export class Router { e.preventDefault(); e.returnValue = ''; } else { + this.save_scroll_state(); + history.scrollRestoration = 'auto'; } }); @@ -105,25 +121,6 @@ export class Router { history.scrollRestoration = 'manual'; }); - // There's no API to capture the scroll location right before the user - // hits the back/forward button, so we listen for scroll events - - /** @type {NodeJS.Timeout} */ - let scroll_timer; - addEventListener('scroll', () => { - clearTimeout(scroll_timer); - scroll_timer = setTimeout(() => { - // Store the scroll location in the history - // This will persist even if we navigate away from the site and come back - const new_state = { - ...(history.state || {}), - 'sveltekit:scroll': scroll_state() - }; - history.replaceState(new_state, document.title, window.location.href); - // iOS scroll event intervals happen between 30-150ms, sometimes around 200ms - }, 200); - }); - /** @param {Event} event */ const trigger_prefetch = (event) => { const a = find_anchor(event); @@ -190,11 +187,10 @@ export class Router { // Removing the hash does a full page navigation in the browser, so make sure a hash is present const [base, hash] = url.href.split('#'); if (hash !== undefined && base === location.href.split('#')[0]) { - // Call `pushState` to add url to history so going back works. - // Also make a delay, otherwise the browser default behaviour would not kick in - setTimeout(() => history.pushState({}, '', url.href)); + this.save_scroll_state(); const info = this.parse(url); if (info) { + history.pushState({}, '', url.href); return this.renderer.update(info, [], false); } return; @@ -377,6 +373,8 @@ export class Router { return; } + this.save_scroll_state(); + const info = this.parse(url); if (!info) { location.href = url.href; From 735dc1c8383aa8d981ba91897139592d310b0129 Mon Sep 17 00:00:00 2001 From: Mihael Date: Sat, 12 Feb 2022 21:02:47 +0100 Subject: [PATCH 02/63] . --- packages/kit/src/runtime/client/router.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index 69e7a14e389a..3e316ada054f 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -187,6 +187,7 @@ export class Router { // Removing the hash does a full page navigation in the browser, so make sure a hash is present const [base, hash] = url.href.split('#'); if (hash !== undefined && base === location.href.split('#')[0]) { + event.preventDefault(); this.save_scroll_state(); const info = this.parse(url); if (info) { From 9b02c51d554bdff833b8667d35b4dddade57698c Mon Sep 17 00:00:00 2001 From: Mihael Date: Sat, 12 Feb 2022 21:13:57 +0100 Subject: [PATCH 03/63] . --- packages/kit/src/runtime/client/router.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index 3e316ada054f..a74520440cf0 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -187,11 +187,9 @@ export class Router { // Removing the hash does a full page navigation in the browser, so make sure a hash is present const [base, hash] = url.href.split('#'); if (hash !== undefined && base === location.href.split('#')[0]) { - event.preventDefault(); this.save_scroll_state(); const info = this.parse(url); if (info) { - history.pushState({}, '', url.href); return this.renderer.update(info, [], false); } return; From e30615ec99f49bb48e4beccb2494f58d4661881c Mon Sep 17 00:00:00 2001 From: Mihael Date: Sat, 12 Feb 2022 21:28:04 +0100 Subject: [PATCH 04/63] . --- packages/kit/src/runtime/client/router.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index a74520440cf0..84a1d8d4582e 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -188,6 +188,9 @@ export class Router { const [base, hash] = url.href.split('#'); if (hash !== undefined && base === location.href.split('#')[0]) { this.save_scroll_state(); + setTimeout(() => { + history.replaceState({}, '', url.href); + }); const info = this.parse(url); if (info) { return this.renderer.update(info, [], false); From 432f4150b326ae35b15f60c5a232746885d6ed1b Mon Sep 17 00:00:00 2001 From: Mihael Date: Sat, 12 Feb 2022 21:38:03 +0100 Subject: [PATCH 05/63] test2 --- packages/kit/src/runtime/client/router.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index 84a1d8d4582e..681d54c284ba 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -185,7 +185,7 @@ export class Router { // Check if new url only differs by hash and use the browser default behavior in that case // This will ensure the `hashchange` event is fired // Removing the hash does a full page navigation in the browser, so make sure a hash is present - const [base, hash] = url.href.split('#'); + /*const [base, hash] = url.href.split('#'); if (hash !== undefined && base === location.href.split('#')[0]) { this.save_scroll_state(); setTimeout(() => { @@ -196,7 +196,7 @@ export class Router { return this.renderer.update(info, [], false); } return; - } + }*/ this._navigate({ url, From 1efd2a7a70821efa59c8856b2c0c6ad02517fb4f Mon Sep 17 00:00:00 2001 From: Mihael Date: Sat, 12 Feb 2022 21:43:11 +0100 Subject: [PATCH 06/63] . --- packages/kit/src/runtime/client/router.js | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index 681d54c284ba..e9ff9763f50f 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -182,22 +182,6 @@ export class Router { // Ignore if has a target if (a instanceof SVGAElement ? a.target.baseVal : a.target) return; - // Check if new url only differs by hash and use the browser default behavior in that case - // This will ensure the `hashchange` event is fired - // Removing the hash does a full page navigation in the browser, so make sure a hash is present - /*const [base, hash] = url.href.split('#'); - if (hash !== undefined && base === location.href.split('#')[0]) { - this.save_scroll_state(); - setTimeout(() => { - history.replaceState({}, '', url.href); - }); - const info = this.parse(url); - if (info) { - return this.renderer.update(info, [], false); - } - return; - }*/ - this._navigate({ url, scroll: a.hasAttribute('sveltekit:noscroll') ? scroll_state() : null, From 1d568899e4b54783061c51d1cbd4478dafb22d5a Mon Sep 17 00:00:00 2001 From: Mihael Date: Sat, 12 Feb 2022 22:05:50 +0100 Subject: [PATCH 07/63] trying option B --- packages/kit/src/runtime/client/router.js | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index e9ff9763f50f..5cbbcb17f560 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -149,7 +149,7 @@ export class Router { addEventListener('sveltekit:trigger_prefetch', trigger_prefetch); /** @param {MouseEvent} event */ - addEventListener('click', (event) => { + addEventListener('click', async (event) => { if (!this.enabled) return; // Adapted from https://github.com/visionmedia/page.js @@ -182,6 +182,22 @@ export class Router { // Ignore if has a target if (a instanceof SVGAElement ? a.target.baseVal : a.target) return; + // Check if new url only differs by hash and use the browser default behavior in that case + // This will ensure the `hashchange` event is fired + // Removing the hash does a full page navigation in the browser, so make sure a hash is present + const [base, hash] = url.href.split('#'); + if (hash !== undefined && base === location.href.split('#')[0]) { + event.preventDefault(); + this.save_scroll_state(); + const info = this.parse(url); + if (info) { + await this.renderer.update(info, [], false); + location.hash = hash; // this will automatically pushState and set the state to null and then trigger hashchange event + this.save_scroll_state(); // by calling this method, it will also replaceState with object instead of null, we could also use history.replaceState({}, '', url.href); + } + return; + } + this._navigate({ url, scroll: a.hasAttribute('sveltekit:noscroll') ? scroll_state() : null, From d35ada5386d17fdea68a305d5e0c235c06e0859b Mon Sep 17 00:00:00 2001 From: Mihael Date: Sat, 12 Feb 2022 22:19:11 +0100 Subject: [PATCH 08/63] option B without event listener removed --- packages/kit/src/runtime/client/router.js | 39 ++++++++++++----------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index 5cbbcb17f560..acc8cb5f3d34 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -72,20 +72,6 @@ export class Router { }; } - save_scroll_state() { - const new_state = { - ...(history.state || {}), - 'sveltekit:scroll': scroll_state() - }; - - // need this in case it was set in previous state - if (new_state['sveltekit:scroll'].x === 0 && new_state['sveltekit:scroll'].y === 0) { - delete new_state['sveltekit:scroll']; - } - - history.replaceState(new_state, document.title, window.location.href); - } - init_listeners() { if ('scrollRestoration' in history) { history.scrollRestoration = 'manual'; @@ -110,8 +96,6 @@ export class Router { e.preventDefault(); e.returnValue = ''; } else { - this.save_scroll_state(); - history.scrollRestoration = 'auto'; } }); @@ -121,6 +105,25 @@ export class Router { history.scrollRestoration = 'manual'; }); + // There's no API to capture the scroll location right before the user + // hits the back/forward button, so we listen for scroll events + + /** @type {NodeJS.Timeout} */ + let scroll_timer; + addEventListener('scroll', () => { + clearTimeout(scroll_timer); + scroll_timer = setTimeout(() => { + // Store the scroll location in the history + // This will persist even if we navigate away from the site and come back + const new_state = { + ...(history.state || {}), + 'sveltekit:scroll': scroll_state() + }; + history.replaceState(new_state, document.title, window.location.href); + // iOS scroll event intervals happen between 30-150ms, sometimes around 200ms + }, 200); + }); + /** @param {Event} event */ const trigger_prefetch = (event) => { const a = find_anchor(event); @@ -149,7 +152,7 @@ export class Router { addEventListener('sveltekit:trigger_prefetch', trigger_prefetch); /** @param {MouseEvent} event */ - addEventListener('click', async (event) => { + addEventListener('click', (event) => { if (!this.enabled) return; // Adapted from https://github.com/visionmedia/page.js @@ -375,8 +378,6 @@ export class Router { return; } - this.save_scroll_state(); - const info = this.parse(url); if (!info) { location.href = url.href; From 5691f4660be8ba243d4fff6207fe7e0f468cd8ff Mon Sep 17 00:00:00 2001 From: Mihael Date: Sat, 12 Feb 2022 22:23:12 +0100 Subject: [PATCH 09/63] . --- packages/kit/src/runtime/client/router.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index acc8cb5f3d34..e1b5202e6ceb 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -152,7 +152,7 @@ export class Router { addEventListener('sveltekit:trigger_prefetch', trigger_prefetch); /** @param {MouseEvent} event */ - addEventListener('click', (event) => { + addEventListener('click', async (event) => { if (!this.enabled) return; // Adapted from https://github.com/visionmedia/page.js @@ -196,7 +196,7 @@ export class Router { if (info) { await this.renderer.update(info, [], false); location.hash = hash; // this will automatically pushState and set the state to null and then trigger hashchange event - this.save_scroll_state(); // by calling this method, it will also replaceState with object instead of null, we could also use history.replaceState({}, '', url.href); + history.replaceState({}, '', url.href); } return; } From 1f6604d01c6fb38b484567c3e39caa68e6713153 Mon Sep 17 00:00:00 2001 From: Mihael Date: Sat, 12 Feb 2022 22:24:24 +0100 Subject: [PATCH 10/63] . --- packages/kit/src/runtime/client/router.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index e1b5202e6ceb..31c7b55ff29f 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -191,7 +191,6 @@ export class Router { const [base, hash] = url.href.split('#'); if (hash !== undefined && base === location.href.split('#')[0]) { event.preventDefault(); - this.save_scroll_state(); const info = this.parse(url); if (info) { await this.renderer.update(info, [], false); From f9ce80fa6772880b70a965a0d1a86f980cbe93af Mon Sep 17 00:00:00 2001 From: Mihael Date: Sun, 13 Feb 2022 13:02:06 +0100 Subject: [PATCH 11/63] wrong attempt --- packages/kit/src/runtime/client/router.js | 67 ++++++++++++++--------- 1 file changed, 40 insertions(+), 27 deletions(-) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index 31c7b55ff29f..e844f2e94381 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -50,6 +50,12 @@ export class Router { this.renderer = renderer; renderer.router = this; + /** @type {{current: string | null, positions: { [key: string]: {x: number, y: number} }}} */ + this.scroll_positions = sessionStorage.getItem('sveltekit:scroll_positions') + ? // @ts-ignore + JSON.parse(sessionStorage.getItem('sveltekit:scroll_positions')) + : { current: null, positions: {} }; + this.enabled = true; // make it possible to reset focus @@ -72,6 +78,29 @@ export class Router { }; } + save_scroll_state(from_popstate = false) { + const current_scroll = scroll_state(); + if (!from_popstate) { + const new_state = history.state || {}; + + // in case X and Y are both 0, we don't need to store them + if (current_scroll.x === 0 && current_scroll.y === 0) { + this.scroll_positions.current = null; + if (new_state['sveltekit:scroll_key']) { + delete this.scroll_positions.positions[new_state['sveltekit:scroll_key']]; + delete new_state['sveltekit:scroll_key']; + } + } else { + new_state['sveltekit:scroll_key'] = Math.random().toString(32).slice(2); + this.scroll_positions.positions[new_state['sveltekit:scroll_key']] = current_scroll; + } + + history.replaceState(new_state, document.title, location.href); + } else { + // TODO + } + } + init_listeners() { if ('scrollRestoration' in history) { history.scrollRestoration = 'manual'; @@ -96,6 +125,8 @@ export class Router { e.preventDefault(); e.returnValue = ''; } else { + // need this in case user navigates away and returns back to the page to be able to restore it + sessionStorage.setItem('sveltekit:scroll_positions', JSON.stringify(this.scroll_positions)); history.scrollRestoration = 'auto'; } }); @@ -105,25 +136,6 @@ export class Router { history.scrollRestoration = 'manual'; }); - // There's no API to capture the scroll location right before the user - // hits the back/forward button, so we listen for scroll events - - /** @type {NodeJS.Timeout} */ - let scroll_timer; - addEventListener('scroll', () => { - clearTimeout(scroll_timer); - scroll_timer = setTimeout(() => { - // Store the scroll location in the history - // This will persist even if we navigate away from the site and come back - const new_state = { - ...(history.state || {}), - 'sveltekit:scroll': scroll_state() - }; - history.replaceState(new_state, document.title, window.location.href); - // iOS scroll event intervals happen between 30-150ms, sometimes around 200ms - }, 200); - }); - /** @param {Event} event */ const trigger_prefetch = (event) => { const a = find_anchor(event); @@ -152,7 +164,7 @@ export class Router { addEventListener('sveltekit:trigger_prefetch', trigger_prefetch); /** @param {MouseEvent} event */ - addEventListener('click', async (event) => { + addEventListener('click', (event) => { if (!this.enabled) return; // Adapted from https://github.com/visionmedia/page.js @@ -190,13 +202,10 @@ export class Router { // Removing the hash does a full page navigation in the browser, so make sure a hash is present const [base, hash] = url.href.split('#'); if (hash !== undefined && base === location.href.split('#')[0]) { - event.preventDefault(); - const info = this.parse(url); - if (info) { - await this.renderer.update(info, [], false); - location.hash = hash; // this will automatically pushState and set the state to null and then trigger hashchange event - history.replaceState({}, '', url.href); - } + event.preventDefault(); // by preventing this click event and later invoking hashchange event on our own (with location.hash = url.hash) we remove the need for using that fiddly setTimeout hack + this.save_scroll_state(false); + location.hash = url.hash; // this will automatically pushState and set the state to null and then trigger hashchange event + this.save_scroll_state(false); // by calling this method, it will also replaceState with object instead of null, we could also use history.replaceState({}, '', url.href); return; } @@ -220,6 +229,8 @@ export class Router { // with history.go, which means we end up back here, hence this check if (event.state['sveltekit:index'] === this.current_history_index) return; + this.save_scroll_state(true); + this._navigate({ url: new URL(location.href), scroll: event.state['sveltekit:scroll'], @@ -377,6 +388,8 @@ export class Router { return; } + this.save_scroll_state(false); + const info = this.parse(url); if (!info) { location.href = url.href; From 55a6049bf8b10e03b45a5f1e56d5590bb855e199 Mon Sep 17 00:00:00 2001 From: Mihael Date: Sun, 13 Feb 2022 13:54:45 +0100 Subject: [PATCH 12/63] . --- packages/kit/src/runtime/client/router.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index e844f2e94381..b85e20cad9b2 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -50,11 +50,11 @@ export class Router { this.renderer = renderer; renderer.router = this; - /** @type {{current: string | null, positions: { [key: string]: {x: number, y: number} }}} */ + /** @type {{current_key: string | null, positions: { [key: string]: {x: number, y: number} }}} */ this.scroll_positions = sessionStorage.getItem('sveltekit:scroll_positions') ? // @ts-ignore JSON.parse(sessionStorage.getItem('sveltekit:scroll_positions')) - : { current: null, positions: {} }; + : { current_key: null, positions: {} }; this.enabled = true; @@ -65,8 +65,10 @@ export class Router { this.current_history_index = history.state?.['sveltekit:index'] ?? 0; if (this.current_history_index === 0) { + const scroll_key = Math.random().toString(32).slice(2); // create initial history entry, so we can return here - history.replaceState({ ...history.state, 'sveltekit:index': 0 }, '', location.href); + history.replaceState({ ...history.state, 'sveltekit:index': 0, 'sveltekit:key': scroll_key }, '', location.href); + this.scroll_positions.current_key = scroll_key; } this.callbacks = { From 36e81e231aca47e8620ff95e33f7a41f408604ac Mon Sep 17 00:00:00 2001 From: Mihael Date: Sun, 13 Feb 2022 14:24:57 +0100 Subject: [PATCH 13/63] scroll restoration overhaul --- .changeset/shy-taxis-drum.md | 2 +- packages/kit/src/runtime/client/router.js | 59 +++++++++-------------- 2 files changed, 24 insertions(+), 37 deletions(-) diff --git a/.changeset/shy-taxis-drum.md b/.changeset/shy-taxis-drum.md index 4867115e5ca0..077fae1e1d64 100644 --- a/.changeset/shy-taxis-drum.md +++ b/.changeset/shy-taxis-drum.md @@ -2,4 +2,4 @@ '@sveltejs/kit': patch --- -fixes scroll navigation issue and removes the need for scroll event listener +scroll restoration overhaul diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index b85e20cad9b2..dd1885b53825 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -50,25 +50,18 @@ export class Router { this.renderer = renderer; renderer.router = this; - /** @type {{current_key: string | null, positions: { [key: string]: {x: number, y: number} }}} */ - this.scroll_positions = sessionStorage.getItem('sveltekit:scroll_positions') - ? // @ts-ignore - JSON.parse(sessionStorage.getItem('sveltekit:scroll_positions')) - : { current_key: null, positions: {} }; - this.enabled = true; // make it possible to reset focus document.body.setAttribute('tabindex', '-1'); // keeping track of the history index in order to prevent popstate navigation events if needed + /** @type {number} */ this.current_history_index = history.state?.['sveltekit:index'] ?? 0; if (this.current_history_index === 0) { - const scroll_key = Math.random().toString(32).slice(2); // create initial history entry, so we can return here - history.replaceState({ ...history.state, 'sveltekit:index': 0, 'sveltekit:key': scroll_key }, '', location.href); - this.scroll_positions.current_key = scroll_key; + history.replaceState({ ...history.state, 'sveltekit:index': 0 }, '', location.href); } this.callbacks = { @@ -80,27 +73,20 @@ export class Router { }; } - save_scroll_state(from_popstate = false) { - const current_scroll = scroll_state(); - if (!from_popstate) { - const new_state = history.state || {}; - - // in case X and Y are both 0, we don't need to store them - if (current_scroll.x === 0 && current_scroll.y === 0) { - this.scroll_positions.current = null; - if (new_state['sveltekit:scroll_key']) { - delete this.scroll_positions.positions[new_state['sveltekit:scroll_key']]; - delete new_state['sveltekit:scroll_key']; - } - } else { - new_state['sveltekit:scroll_key'] = Math.random().toString(32).slice(2); - this.scroll_positions.positions[new_state['sveltekit:scroll_key']] = current_scroll; - } + save_scroll_position() { + sessionStorage.setItem( + `sveltekit:scroll:${this.current_history_index}`, + JSON.stringify(scroll_state()) + ); + } - history.replaceState(new_state, document.title, location.href); - } else { - // TODO + /** @param {number} index */ + get_scroll_position(index) { + const scroll = sessionStorage.getItem(`sveltekit:scroll:${index}`); + if (scroll) { + return JSON.parse(scroll); } + return null; } init_listeners() { @@ -127,8 +113,6 @@ export class Router { e.preventDefault(); e.returnValue = ''; } else { - // need this in case user navigates away and returns back to the page to be able to restore it - sessionStorage.setItem('sveltekit:scroll_positions', JSON.stringify(this.scroll_positions)); history.scrollRestoration = 'auto'; } }); @@ -205,9 +189,13 @@ export class Router { const [base, hash] = url.href.split('#'); if (hash !== undefined && base === location.href.split('#')[0]) { event.preventDefault(); // by preventing this click event and later invoking hashchange event on our own (with location.hash = url.hash) we remove the need for using that fiddly setTimeout hack - this.save_scroll_state(false); + this.save_scroll_position(); location.hash = url.hash; // this will automatically pushState and set the state to null and then trigger hashchange event - this.save_scroll_state(false); // by calling this method, it will also replaceState with object instead of null, we could also use history.replaceState({}, '', url.href); + history.replaceState( + { ...(history.state || {}), 'sveltekit:index': ++this.current_history_index }, + '', + location.href + ); return; } @@ -231,11 +219,11 @@ export class Router { // with history.go, which means we end up back here, hence this check if (event.state['sveltekit:index'] === this.current_history_index) return; - this.save_scroll_state(true); + this.save_scroll_position(); this._navigate({ url: new URL(location.href), - scroll: event.state['sveltekit:scroll'], + scroll: this.get_scroll_position(event.state['sveltekit:index']), keepfocus: false, chain: [], details: null, @@ -390,8 +378,6 @@ export class Router { return; } - this.save_scroll_state(false); - const info = this.parse(url); if (!info) { location.href = url.href; @@ -412,6 +398,7 @@ export class Router { info.url = new URL(url.origin + pathname + url.search + url.hash); if (details) { + this.save_scroll_position(); const change = details.replaceState ? 0 : 1; details.state['sveltekit:index'] = this.current_history_index += change; history[details.replaceState ? 'replaceState' : 'pushState'](details.state, '', info.url); From b955f03e3339907ba0edd57915acf33eeeb93bb1 Mon Sep 17 00:00:00 2001 From: Mihael Date: Sun, 13 Feb 2022 14:40:17 +0100 Subject: [PATCH 14/63] . --- packages/kit/src/runtime/client/router.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index dd1885b53825..0dd5f4c72df3 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -386,6 +386,7 @@ export class Router { }); } + this.save_scroll_position(); accepted(); if (!this.navigating) { @@ -398,7 +399,6 @@ export class Router { info.url = new URL(url.origin + pathname + url.search + url.hash); if (details) { - this.save_scroll_position(); const change = details.replaceState ? 0 : 1; details.state['sveltekit:index'] = this.current_history_index += change; history[details.replaceState ? 'replaceState' : 'pushState'](details.state, '', info.url); From d5fe97968551f09c8e3e5b40ebaa41fe6e060b84 Mon Sep 17 00:00:00 2001 From: Mihael Date: Sun, 13 Feb 2022 15:02:40 +0100 Subject: [PATCH 15/63] test issues fixed --- packages/kit/src/runtime/client/router.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index 0dd5f4c72df3..7b42fa9c3cc6 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -196,6 +196,10 @@ export class Router { '', location.href ); + const unsubscribe = this.renderer.stores.page.subscribe((page) => { + unsubscribe(); + this.renderer.stores.page.set({ ...page, url: new URL(url.href) }); + }); return; } From b5a3a20d24a78f01784642e7b51139f070137c34 Mon Sep 17 00:00:00 2001 From: Mihael Date: Sun, 13 Feb 2022 15:12:12 +0100 Subject: [PATCH 16/63] . --- packages/kit/src/runtime/client/router.js | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index 7b42fa9c3cc6..e3e1071bcb08 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -188,18 +188,19 @@ export class Router { // Removing the hash does a full page navigation in the browser, so make sure a hash is present const [base, hash] = url.href.split('#'); if (hash !== undefined && base === location.href.split('#')[0]) { - event.preventDefault(); // by preventing this click event and later invoking hashchange event on our own (with location.hash = url.hash) we remove the need for using that fiddly setTimeout hack - this.save_scroll_position(); - location.hash = url.hash; // this will automatically pushState and set the state to null and then trigger hashchange event - history.replaceState( - { ...(history.state || {}), 'sveltekit:index': ++this.current_history_index }, - '', - location.href - ); - const unsubscribe = this.renderer.stores.page.subscribe((page) => { - unsubscribe(); - this.renderer.stores.page.set({ ...page, url: new URL(url.href) }); + // Call `pushState` to add url to history so going back works. + // Also make a delay, otherwise the browser default behaviour would not kick in + setTimeout(() => { + history.replaceState( + { ...(history.state || {}), 'sveltekit:index': ++this.current_history_index }, + '', + location.href + ); }); + const info = this.parse(url); + if (info) { + return this.renderer.update(info, [], false); + } return; } From 2f9adebe3aa5ec02bb08c172ae655b21cce4ffd4 Mon Sep 17 00:00:00 2001 From: Mihael Date: Sun, 13 Feb 2022 15:14:54 +0100 Subject: [PATCH 17/63] . --- packages/kit/src/runtime/client/router.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index e3e1071bcb08..959fbdeafd6c 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -188,8 +188,8 @@ export class Router { // Removing the hash does a full page navigation in the browser, so make sure a hash is present const [base, hash] = url.href.split('#'); if (hash !== undefined && base === location.href.split('#')[0]) { - // Call `pushState` to add url to history so going back works. - // Also make a delay, otherwise the browser default behaviour would not kick in + // Call `replaceState` in timeout because this event will add pushState automatically and we need to store our own history state + // Timeout is being used to delay replaceState till next tick setTimeout(() => { history.replaceState( { ...(history.state || {}), 'sveltekit:index': ++this.current_history_index }, From b2b906fb5bff1d7e363ca3b15cd10c54ef01bcd5 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Sun, 13 Feb 2022 08:53:24 -0800 Subject: [PATCH 18/63] uncomment previously failing test --- packages/kit/test/apps/basics/test/test.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index 729ac83dc188..c3e31270b8d5 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -214,11 +214,9 @@ test.describe('Scrolling', () => { expect(page.url()).toBe(baseURL + '/anchor#last-anchor-2'); const scrollY = /** @type {number} */ (await page.evaluate(() => scrollY)); expect(scrollY).toEqual(originalScrollY); - // TODO: fix this. it is failing due to duplicate history entries - // https://github.com/sveltejs/kit/issues/3636 - // await page.goBack(); - // expect(page.url()).toBe(baseURL + '/anchor'); - // expect(scrollY).toEqual(0); + await page.goBack(); + expect(page.url()).toBe(baseURL + '/anchor'); + expect(scrollY).toEqual(0); }); test('url-supplied anchor is ignored with onMount() scrolling on direct page load', async ({ From 3a254006ae349aa75b1d763d4617daaf844ac256 Mon Sep 17 00:00:00 2001 From: Mihael Date: Sun, 13 Feb 2022 18:07:08 +0100 Subject: [PATCH 19/63] . --- packages/kit/src/runtime/client/router.js | 10 ++++++++-- packages/kit/test/apps/basics/test/test.js | 5 ++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index 959fbdeafd6c..2c1dfa3ba4e3 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -80,13 +80,19 @@ export class Router { ); } - /** @param {number} index */ + /** + * + * @param {number} index + * @returns {{x: number, y: number} | undefined} + */ get_scroll_position(index) { + if (index == undefined) { + return; + } const scroll = sessionStorage.getItem(`sveltekit:scroll:${index}`); if (scroll) { return JSON.parse(scroll); } - return null; } init_listeners() { diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index c3e31270b8d5..e20a27fdf711 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -212,11 +212,10 @@ test.describe('Scrolling', () => { await clicknav('#routing-page'); await back(); expect(page.url()).toBe(baseURL + '/anchor#last-anchor-2'); - const scrollY = /** @type {number} */ (await page.evaluate(() => scrollY)); - expect(scrollY).toEqual(originalScrollY); + expect(/** @type {number} */ (await page.evaluate(() => scrollY))).toEqual(originalScrollY); await page.goBack(); expect(page.url()).toBe(baseURL + '/anchor'); - expect(scrollY).toEqual(0); + expect(/** @type {number} */ (await page.evaluate(() => scrollY))).toEqual(0); }); test('url-supplied anchor is ignored with onMount() scrolling on direct page load', async ({ From 605210f114f36af04f3d74d325bd254ffa80e1bb Mon Sep 17 00:00:00 2001 From: Mihael Date: Sun, 13 Feb 2022 18:23:42 +0100 Subject: [PATCH 20/63] Revert "uncomment previously failing test" This reverts commit b2b906fb5bff1d7e363ca3b15cd10c54ef01bcd5. --- packages/kit/test/apps/basics/test/test.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index e20a27fdf711..7a808b08beb3 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -212,10 +212,20 @@ test.describe('Scrolling', () => { await clicknav('#routing-page'); await back(); expect(page.url()).toBe(baseURL + '/anchor#last-anchor-2'); +<<<<<<< HEAD expect(/** @type {number} */ (await page.evaluate(() => scrollY))).toEqual(originalScrollY); await page.goBack(); expect(page.url()).toBe(baseURL + '/anchor'); expect(/** @type {number} */ (await page.evaluate(() => scrollY))).toEqual(0); +======= + const scrollY = /** @type {number} */ (await page.evaluate(() => scrollY)); + expect(scrollY).toEqual(originalScrollY); + // TODO: fix this. it is failing due to duplicate history entries + // https://github.com/sveltejs/kit/issues/3636 + // await page.goBack(); + // expect(page.url()).toBe(baseURL + '/anchor'); + // expect(scrollY).toEqual(0); +>>>>>>> parent of b2b906fb (uncomment previously failing test) }); test('url-supplied anchor is ignored with onMount() scrolling on direct page load', async ({ From 14e5e53cacec03aba1b623d6a9d8ddc143a6a198 Mon Sep 17 00:00:00 2001 From: Mihael Date: Sun, 13 Feb 2022 18:25:23 +0100 Subject: [PATCH 21/63] type --- packages/kit/src/runtime/client/router.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index 2c1dfa3ba4e3..071b2226194e 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -83,15 +83,17 @@ export class Router { /** * * @param {number} index - * @returns {{x: number, y: number} | undefined} + * @returns {{x: number, y: number} | null} */ get_scroll_position(index) { if (index == undefined) { - return; + return null; } const scroll = sessionStorage.getItem(`sveltekit:scroll:${index}`); if (scroll) { return JSON.parse(scroll); + } else { + return null; } } From 375c1347393e0750dd29ddc596ffe44d2af1edce Mon Sep 17 00:00:00 2001 From: Mihael Date: Sun, 13 Feb 2022 18:27:09 +0100 Subject: [PATCH 22/63] . --- packages/kit/test/apps/basics/test/test.js | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index 7a808b08beb3..99b699212d82 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -212,12 +212,6 @@ test.describe('Scrolling', () => { await clicknav('#routing-page'); await back(); expect(page.url()).toBe(baseURL + '/anchor#last-anchor-2'); -<<<<<<< HEAD - expect(/** @type {number} */ (await page.evaluate(() => scrollY))).toEqual(originalScrollY); - await page.goBack(); - expect(page.url()).toBe(baseURL + '/anchor'); - expect(/** @type {number} */ (await page.evaluate(() => scrollY))).toEqual(0); -======= const scrollY = /** @type {number} */ (await page.evaluate(() => scrollY)); expect(scrollY).toEqual(originalScrollY); // TODO: fix this. it is failing due to duplicate history entries @@ -225,7 +219,6 @@ test.describe('Scrolling', () => { // await page.goBack(); // expect(page.url()).toBe(baseURL + '/anchor'); // expect(scrollY).toEqual(0); ->>>>>>> parent of b2b906fb (uncomment previously failing test) }); test('url-supplied anchor is ignored with onMount() scrolling on direct page load', async ({ @@ -2066,4 +2059,4 @@ test.describe.parallel('XSS', () => { 'user.name is ' ); }); -}); +}); \ No newline at end of file From effbdbf5c8acae07824d235471cbafecf84187a4 Mon Sep 17 00:00:00 2001 From: Mihael Date: Sun, 13 Feb 2022 18:28:32 +0100 Subject: [PATCH 23/63] . --- packages/kit/test/apps/basics/test/test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index 99b699212d82..729ac83dc188 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -2059,4 +2059,4 @@ test.describe.parallel('XSS', () => { 'user.name is ' ); }); -}); \ No newline at end of file +}); From a2e9ad983027090149490b267eaa88dbd1b5285f Mon Sep 17 00:00:00 2001 From: Mihael Date: Sun, 13 Feb 2022 18:32:38 +0100 Subject: [PATCH 24/63] try to fix test --- packages/kit/test/apps/basics/test/test.js | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index 729ac83dc188..aad33d857e44 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -212,13 +212,10 @@ test.describe('Scrolling', () => { await clicknav('#routing-page'); await back(); expect(page.url()).toBe(baseURL + '/anchor#last-anchor-2'); - const scrollY = /** @type {number} */ (await page.evaluate(() => scrollY)); - expect(scrollY).toEqual(originalScrollY); - // TODO: fix this. it is failing due to duplicate history entries - // https://github.com/sveltejs/kit/issues/3636 - // await page.goBack(); - // expect(page.url()).toBe(baseURL + '/anchor'); - // expect(scrollY).toEqual(0); + expect(await page.evaluate(() => scrollY)).toEqual(originalScrollY); + await page.goBack(); + expect(page.url()).toBe(baseURL + '/anchor'); + expect(await page.evaluate(() => scrollY)).toEqual(0); }); test('url-supplied anchor is ignored with onMount() scrolling on direct page load', async ({ From 425591277e21a19503501c6035e8a628ee6f74e9 Mon Sep 17 00:00:00 2001 From: Mihael Date: Sun, 13 Feb 2022 18:43:22 +0100 Subject: [PATCH 25/63] rerun test From 75175c59a27f7d950533df0303362d229cecbbd4 Mon Sep 17 00:00:00 2001 From: Mihael Date: Sun, 13 Feb 2022 18:57:31 +0100 Subject: [PATCH 26/63] fix --- packages/kit/src/runtime/client/router.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index 071b2226194e..bb9693f96e78 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -196,6 +196,8 @@ export class Router { // Removing the hash does a full page navigation in the browser, so make sure a hash is present const [base, hash] = url.href.split('#'); if (hash !== undefined && base === location.href.split('#')[0]) { + this.save_scroll_position(); + // Call `replaceState` in timeout because this event will add pushState automatically and we need to store our own history state // Timeout is being used to delay replaceState till next tick setTimeout(() => { From 4822069a3c35a0d7286ca4d73450fffb33e27e0b Mon Sep 17 00:00:00 2001 From: Mihael Date: Sun, 13 Feb 2022 20:20:23 +0100 Subject: [PATCH 27/63] trying with setting stores manually --- packages/kit/src/runtime/client/router.js | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index bb9693f96e78..890b29adaffa 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -196,21 +196,18 @@ export class Router { // Removing the hash does a full page navigation in the browser, so make sure a hash is present const [base, hash] = url.href.split('#'); if (hash !== undefined && base === location.href.split('#')[0]) { + event.preventDefault(); this.save_scroll_position(); - // Call `replaceState` in timeout because this event will add pushState automatically and we need to store our own history state - // Timeout is being used to delay replaceState till next tick - setTimeout(() => { + const unsubscribe = this.renderer.stores.page.subscribe((page) => { + unsubscribe(); + this.renderer.stores.page.set({ ...page, url: new URL(url.href) }); history.replaceState( { ...(history.state || {}), 'sveltekit:index': ++this.current_history_index }, '', - location.href + url.href ); }); - const info = this.parse(url); - if (info) { - return this.renderer.update(info, [], false); - } return; } From 8e27aec36eae7d2f21e4b80635d57ab846a89e09 Mon Sep 17 00:00:00 2001 From: Mihael Date: Sun, 13 Feb 2022 20:21:59 +0100 Subject: [PATCH 28/63] . --- packages/kit/src/runtime/client/router.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index 890b29adaffa..74d1e22a3edd 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -202,6 +202,7 @@ export class Router { const unsubscribe = this.renderer.stores.page.subscribe((page) => { unsubscribe(); this.renderer.stores.page.set({ ...page, url: new URL(url.href) }); + location.hash = url.hash; history.replaceState( { ...(history.state || {}), 'sveltekit:index': ++this.current_history_index }, '', From 159567e6403aa86a9bbde4cf1ec240f34fa7f1c6 Mon Sep 17 00:00:00 2001 From: Mihael Date: Sun, 13 Feb 2022 20:28:46 +0100 Subject: [PATCH 29/63] testing --- packages/kit/src/runtime/client/router.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index 74d1e22a3edd..0b21e3b15185 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -200,6 +200,7 @@ export class Router { this.save_scroll_position(); const unsubscribe = this.renderer.stores.page.subscribe((page) => { + console.log({page}); // testing unsubscribe(); this.renderer.stores.page.set({ ...page, url: new URL(url.href) }); location.hash = url.hash; From 708170f3618da2e26851c085903448baae67fd8c Mon Sep 17 00:00:00 2001 From: Mihael Date: Sun, 13 Feb 2022 20:31:16 +0100 Subject: [PATCH 30/63] lint --- packages/kit/src/runtime/client/router.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index 0b21e3b15185..848cbd710104 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -200,7 +200,7 @@ export class Router { this.save_scroll_position(); const unsubscribe = this.renderer.stores.page.subscribe((page) => { - console.log({page}); // testing + console.log({ page }); // testing unsubscribe(); this.renderer.stores.page.set({ ...page, url: new URL(url.href) }); location.hash = url.hash; From 4b2df43fa5bd41c552be629f38967c2c91c6f10c Mon Sep 17 00:00:00 2001 From: Mihael Date: Sun, 13 Feb 2022 20:38:56 +0100 Subject: [PATCH 31/63] clg this --- packages/kit/src/runtime/client/router.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index 848cbd710104..973ae1d3b95e 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -196,6 +196,7 @@ export class Router { // Removing the hash does a full page navigation in the browser, so make sure a hash is present const [base, hash] = url.href.split('#'); if (hash !== undefined && base === location.href.split('#')[0]) { + console.log(this); event.preventDefault(); this.save_scroll_position(); From af167efb78c35a1ec92554ce60788054059c7fae Mon Sep 17 00:00:00 2001 From: Mihael Date: Sun, 13 Feb 2022 20:48:52 +0100 Subject: [PATCH 32/63] . --- packages/kit/src/runtime/client/router.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index 973ae1d3b95e..664f92bf45ea 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -202,7 +202,6 @@ export class Router { const unsubscribe = this.renderer.stores.page.subscribe((page) => { console.log({ page }); // testing - unsubscribe(); this.renderer.stores.page.set({ ...page, url: new URL(url.href) }); location.hash = url.hash; history.replaceState( @@ -211,6 +210,7 @@ export class Router { url.href ); }); + setTimeout(unsubscribe, 20); return; } From 5c3fc29742e824c482147f609e0a5c27eb29f269 Mon Sep 17 00:00:00 2001 From: Mihael Date: Sun, 13 Feb 2022 20:54:43 +0100 Subject: [PATCH 33/63] . --- packages/kit/src/runtime/client/router.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index 664f92bf45ea..8afce0126068 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -209,8 +209,9 @@ export class Router { '', url.href ); + unsubscribe(); }); - setTimeout(unsubscribe, 20); + //setTimeout(unsubscribe, 20); return; } From 2da7e9e115247ae5cf3ee48ce1206740c9c0ab66 Mon Sep 17 00:00:00 2001 From: Mihael Date: Sun, 13 Feb 2022 21:02:19 +0100 Subject: [PATCH 34/63] . --- packages/kit/src/runtime/client/router.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index 8afce0126068..36b4ca724f0f 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -1,4 +1,4 @@ -import { onMount } from 'svelte'; +import { onMount, tick } from 'svelte'; import { normalize_path } from '../../utils/url'; import { get_base_uri } from './utils'; @@ -196,12 +196,10 @@ export class Router { // Removing the hash does a full page navigation in the browser, so make sure a hash is present const [base, hash] = url.href.split('#'); if (hash !== undefined && base === location.href.split('#')[0]) { - console.log(this); event.preventDefault(); this.save_scroll_position(); const unsubscribe = this.renderer.stores.page.subscribe((page) => { - console.log({ page }); // testing this.renderer.stores.page.set({ ...page, url: new URL(url.href) }); location.hash = url.hash; history.replaceState( @@ -209,9 +207,9 @@ export class Router { '', url.href ); - unsubscribe(); + // @ts-ignore + tick.then(unsubscribe); }); - //setTimeout(unsubscribe, 20); return; } From 37735ab38a96735ffdd358e48d997147407cfc2c Mon Sep 17 00:00:00 2001 From: Mihael Date: Sun, 13 Feb 2022 21:08:55 +0100 Subject: [PATCH 35/63] try --- packages/kit/src/runtime/client/router.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index 36b4ca724f0f..44af7a612770 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -199,7 +199,10 @@ export class Router { event.preventDefault(); this.save_scroll_position(); - const unsubscribe = this.renderer.stores.page.subscribe((page) => { + /** @type {function} */ + let unsubscribe; + // eslint-disable-next-line prefer-const + unsubscribe = this.renderer.stores.page.subscribe((page) => { this.renderer.stores.page.set({ ...page, url: new URL(url.href) }); location.hash = url.hash; history.replaceState( From e5986b0cf662ee565d45e1a7ac1572cd6376dbb7 Mon Sep 17 00:00:00 2001 From: Mihael Date: Sun, 13 Feb 2022 21:13:10 +0100 Subject: [PATCH 36/63] . --- packages/kit/src/runtime/client/router.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index 44af7a612770..792902e63560 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -1,4 +1,4 @@ -import { onMount, tick } from 'svelte'; +import { onMount } from 'svelte'; import { normalize_path } from '../../utils/url'; import { get_base_uri } from './utils'; @@ -211,7 +211,7 @@ export class Router { url.href ); // @ts-ignore - tick.then(unsubscribe); + Promise.resolve().then(unsubscribe); }); return; } From ec41da6bb22d9e1d58d85f1ef1d1f843f1690f7f Mon Sep 17 00:00:00 2001 From: Mihael Date: Sun, 13 Feb 2022 21:20:18 +0100 Subject: [PATCH 37/63] working --- packages/kit/src/runtime/client/router.js | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index 792902e63560..bb9693f96e78 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -196,23 +196,21 @@ export class Router { // Removing the hash does a full page navigation in the browser, so make sure a hash is present const [base, hash] = url.href.split('#'); if (hash !== undefined && base === location.href.split('#')[0]) { - event.preventDefault(); this.save_scroll_position(); - /** @type {function} */ - let unsubscribe; - // eslint-disable-next-line prefer-const - unsubscribe = this.renderer.stores.page.subscribe((page) => { - this.renderer.stores.page.set({ ...page, url: new URL(url.href) }); - location.hash = url.hash; + // Call `replaceState` in timeout because this event will add pushState automatically and we need to store our own history state + // Timeout is being used to delay replaceState till next tick + setTimeout(() => { history.replaceState( { ...(history.state || {}), 'sveltekit:index': ++this.current_history_index }, '', - url.href + location.href ); - // @ts-ignore - Promise.resolve().then(unsubscribe); }); + const info = this.parse(url); + if (info) { + return this.renderer.update(info, [], false); + } return; } From cf7f9b5aca4c95bf4e14bcba220c7f86265c4140 Mon Sep 17 00:00:00 2001 From: Mihael Date: Mon, 14 Feb 2022 11:13:33 +0100 Subject: [PATCH 38/63] empty commit to trigger test From fa62c5f75b6c6399989c96b1d06b50e270c7985f Mon Sep 17 00:00:00 2001 From: Mihael Date: Mon, 14 Feb 2022 11:32:11 +0100 Subject: [PATCH 39/63] trying to change in_view --- packages/kit/test/utils.js | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/packages/kit/test/utils.js b/packages/kit/test/utils.js index a4fda1c72da1..b080f8f7bade 100644 --- a/packages/kit/test/utils.js +++ b/packages/kit/test/utils.js @@ -111,9 +111,21 @@ export const test = base.extend({ in_view: async ({ page }, use) => { /** @param {string} selector */ async function in_view(selector) { - const box = await page.locator(selector).boundingBox(); - const view = await page.viewportSize(); - return box && view && box.y < view.height && box.y + box.height > 0; + // source: https://stackoverflow.com/a/68848306 + return await page.$eval(selector, async (/** @type {Element} */ element) => { + /** @type {number} */ + const visibleRatio = await new Promise((resolve) => { + const observer = new IntersectionObserver((entries) => { + resolve(entries[0].intersectionRatio); + observer.disconnect(); + }); + observer.observe(element); + // Firefox doesn't call IntersectionObserver callback unless + // there are rafs. + requestAnimationFrame(() => {}); + }); + return visibleRatio > 0; + }); } use(in_view); From f39b81aa45c79bc6add38c53419079ab8b3d2fc5 Mon Sep 17 00:00:00 2001 From: Mihael Date: Mon, 14 Feb 2022 11:46:50 +0100 Subject: [PATCH 40/63] test passed, trying again, empty commit From 366fe319080c1b1de88b308e5989bb1276a0837d Mon Sep 17 00:00:00 2001 From: Mihael Date: Mon, 14 Feb 2022 13:00:58 +0100 Subject: [PATCH 41/63] trying another in_view function --- packages/kit/test/utils.js | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/packages/kit/test/utils.js b/packages/kit/test/utils.js index b080f8f7bade..e29dc1b21ede 100644 --- a/packages/kit/test/utils.js +++ b/packages/kit/test/utils.js @@ -111,21 +111,25 @@ export const test = base.extend({ in_view: async ({ page }, use) => { /** @param {string} selector */ async function in_view(selector) { - // source: https://stackoverflow.com/a/68848306 - return await page.$eval(selector, async (/** @type {Element} */ element) => { - /** @type {number} */ - const visibleRatio = await new Promise((resolve) => { - const observer = new IntersectionObserver((entries) => { - resolve(entries[0].intersectionRatio); - observer.disconnect(); - }); - observer.observe(element); - // Firefox doesn't call IntersectionObserver callback unless - // there are rafs. - requestAnimationFrame(() => {}); - }); - return visibleRatio > 0; - }); + // source: https://stackoverflow.com/a/70174877 + let isVisible = false; + await page.evaluate((/** @type {string} */ selector) => { + const element = document.querySelector(selector); + if (element) { + const rect = element.getBoundingClientRect(); + if (rect.top >= 0 && rect.left >= 0) { + const vw = Math.max(document.documentElement.clientWidth || 0, window.innerWidth || 0); + const vh = Math.max( + document.documentElement.clientHeight || 0, + window.innerHeight || 0 + ); + if (rect.right <= vw && rect.bottom <= vh) { + isVisible = true; + } + } + } + }, selector); + return isVisible; } use(in_view); From e6c830df8fe30cd0434fdb49c42ca2580fc3761f Mon Sep 17 00:00:00 2001 From: Mihael Date: Mon, 14 Feb 2022 13:11:02 +0100 Subject: [PATCH 42/63] clg rect --- packages/kit/test/utils.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/kit/test/utils.js b/packages/kit/test/utils.js index e29dc1b21ede..1fa9c8e76c09 100644 --- a/packages/kit/test/utils.js +++ b/packages/kit/test/utils.js @@ -117,6 +117,7 @@ export const test = base.extend({ const element = document.querySelector(selector); if (element) { const rect = element.getBoundingClientRect(); + console.log({ rect }); if (rect.top >= 0 && rect.left >= 0) { const vw = Math.max(document.documentElement.clientWidth || 0, window.innerWidth || 0); const vh = Math.max( From bad40a43b36f46b98cadca9d28ecb4988baf3b26 Mon Sep 17 00:00:00 2001 From: Mihael Date: Mon, 14 Feb 2022 13:28:22 +0100 Subject: [PATCH 43/63] . --- packages/kit/test/utils.js | 35 +++++++++++++++-------------------- 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/packages/kit/test/utils.js b/packages/kit/test/utils.js index 1fa9c8e76c09..b080f8f7bade 100644 --- a/packages/kit/test/utils.js +++ b/packages/kit/test/utils.js @@ -111,26 +111,21 @@ export const test = base.extend({ in_view: async ({ page }, use) => { /** @param {string} selector */ async function in_view(selector) { - // source: https://stackoverflow.com/a/70174877 - let isVisible = false; - await page.evaluate((/** @type {string} */ selector) => { - const element = document.querySelector(selector); - if (element) { - const rect = element.getBoundingClientRect(); - console.log({ rect }); - if (rect.top >= 0 && rect.left >= 0) { - const vw = Math.max(document.documentElement.clientWidth || 0, window.innerWidth || 0); - const vh = Math.max( - document.documentElement.clientHeight || 0, - window.innerHeight || 0 - ); - if (rect.right <= vw && rect.bottom <= vh) { - isVisible = true; - } - } - } - }, selector); - return isVisible; + // source: https://stackoverflow.com/a/68848306 + return await page.$eval(selector, async (/** @type {Element} */ element) => { + /** @type {number} */ + const visibleRatio = await new Promise((resolve) => { + const observer = new IntersectionObserver((entries) => { + resolve(entries[0].intersectionRatio); + observer.disconnect(); + }); + observer.observe(element); + // Firefox doesn't call IntersectionObserver callback unless + // there are rafs. + requestAnimationFrame(() => {}); + }); + return visibleRatio > 0; + }); } use(in_view); From 3d7d389efcd43951003d70c23f9fd92850d04d45 Mon Sep 17 00:00:00 2001 From: Mihael Date: Mon, 14 Feb 2022 13:42:33 +0100 Subject: [PATCH 44/63] . --- packages/kit/test/utils.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/kit/test/utils.js b/packages/kit/test/utils.js index b080f8f7bade..3535f7a0c9bd 100644 --- a/packages/kit/test/utils.js +++ b/packages/kit/test/utils.js @@ -120,9 +120,8 @@ export const test = base.extend({ observer.disconnect(); }); observer.observe(element); - // Firefox doesn't call IntersectionObserver callback unless - // there are rafs. - requestAnimationFrame(() => {}); + // Firefox doesn't call IntersectionObserver callback unless there are rafs. + // requestAnimationFrame(() => {}); }); return visibleRatio > 0; }); From dd0a1f64bf8b7ba3e629cb2540881fd149cabc40 Mon Sep 17 00:00:00 2001 From: Mihael Date: Mon, 14 Feb 2022 14:04:09 +0100 Subject: [PATCH 45/63] trying the old trick --- packages/kit/test/utils.js | 37 +++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/packages/kit/test/utils.js b/packages/kit/test/utils.js index 3535f7a0c9bd..405d3d2b8d8d 100644 --- a/packages/kit/test/utils.js +++ b/packages/kit/test/utils.js @@ -111,19 +111,32 @@ export const test = base.extend({ in_view: async ({ page }, use) => { /** @param {string} selector */ async function in_view(selector) { - // source: https://stackoverflow.com/a/68848306 return await page.$eval(selector, async (/** @type {Element} */ element) => { - /** @type {number} */ - const visibleRatio = await new Promise((resolve) => { - const observer = new IntersectionObserver((entries) => { - resolve(entries[0].intersectionRatio); - observer.disconnect(); - }); - observer.observe(element); - // Firefox doesn't call IntersectionObserver callback unless there are rafs. - // requestAnimationFrame(() => {}); - }); - return visibleRatio > 0; + // @ts-ignore + let top = element.offsetTop; + // @ts-ignore + let left = element.offsetLeft; + // @ts-ignore + const width = element.offsetWidth; + // @ts-ignore + const height = element.offsetHeight; + + // @ts-ignore + while (element.offsetParent) { + // @ts-ignore + element = element.offsetParent; + // @ts-ignore + top += element.offsetTop; + // @ts-ignore + left += element.offsetLeft; + } + + return top < window.pageYOffset + window.innerHeight && + left < window.pageXOffset + window.innerWidth && + top + height > window.pageYOffset && + left + width > window.pageXOffset + ? true + : { top, left, width, height }; }); } From 44d2636583672de24537d60a9679200e4a3bd232 Mon Sep 17 00:00:00 2001 From: Mihael Date: Mon, 14 Feb 2022 14:23:05 +0100 Subject: [PATCH 46/63] empty commit, retest From d9373a30deb4bb5dc2cd40aac58f6e5a1d49e39e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mihael=20Muti=C4=87?= <35368241+PH4NTOMiki@users.noreply.github.com> Date: Mon, 14 Feb 2022 17:40:22 +0100 Subject: [PATCH 47/63] Update packages/kit/src/runtime/client/router.js Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com> --- packages/kit/src/runtime/client/router.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index bb9693f96e78..e964dc4db802 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -81,7 +81,6 @@ export class Router { } /** - * * @param {number} index * @returns {{x: number, y: number} | null} */ From f986ce73c5567aba86b651ec2299cd182b50becd Mon Sep 17 00:00:00 2001 From: Mihael Date: Tue, 15 Feb 2022 10:54:54 +0100 Subject: [PATCH 48/63] revert tests and test utils, they should be changed in separate PR --- packages/kit/test/apps/basics/test/test.js | 5 ++-- packages/kit/test/utils.js | 31 +++------------------- 2 files changed, 6 insertions(+), 30 deletions(-) diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index aad33d857e44..c3e31270b8d5 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -212,10 +212,11 @@ test.describe('Scrolling', () => { await clicknav('#routing-page'); await back(); expect(page.url()).toBe(baseURL + '/anchor#last-anchor-2'); - expect(await page.evaluate(() => scrollY)).toEqual(originalScrollY); + const scrollY = /** @type {number} */ (await page.evaluate(() => scrollY)); + expect(scrollY).toEqual(originalScrollY); await page.goBack(); expect(page.url()).toBe(baseURL + '/anchor'); - expect(await page.evaluate(() => scrollY)).toEqual(0); + expect(scrollY).toEqual(0); }); test('url-supplied anchor is ignored with onMount() scrolling on direct page load', async ({ diff --git a/packages/kit/test/utils.js b/packages/kit/test/utils.js index 405d3d2b8d8d..ae116dc5f139 100644 --- a/packages/kit/test/utils.js +++ b/packages/kit/test/utils.js @@ -111,33 +111,9 @@ export const test = base.extend({ in_view: async ({ page }, use) => { /** @param {string} selector */ async function in_view(selector) { - return await page.$eval(selector, async (/** @type {Element} */ element) => { - // @ts-ignore - let top = element.offsetTop; - // @ts-ignore - let left = element.offsetLeft; - // @ts-ignore - const width = element.offsetWidth; - // @ts-ignore - const height = element.offsetHeight; - - // @ts-ignore - while (element.offsetParent) { - // @ts-ignore - element = element.offsetParent; - // @ts-ignore - top += element.offsetTop; - // @ts-ignore - left += element.offsetLeft; - } - - return top < window.pageYOffset + window.innerHeight && - left < window.pageXOffset + window.innerWidth && - top + height > window.pageYOffset && - left + width > window.pageXOffset - ? true - : { top, left, width, height }; - }); + const box = await page.locator(selector).boundingBox(); + const view = await page.viewportSize(); + return box && view && box.y < view.height && box.y + box.height > 0; } use(in_view); @@ -151,7 +127,6 @@ export const test = base.extend({ setTimeout(() => { reject(new Error('Timed out')); }, 5000); - addEventListener('sveltekit:start', () => { fulfil(); }); From 61606c23a3e587faa58d923b01da72856a868383 Mon Sep 17 00:00:00 2001 From: Mihael Date: Tue, 15 Feb 2022 11:00:08 +0100 Subject: [PATCH 49/63] revert test --- packages/kit/test/apps/basics/test/test.js | 8 +++++--- packages/kit/test/utils.js | 1 + 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index c3e31270b8d5..729ac83dc188 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -214,9 +214,11 @@ test.describe('Scrolling', () => { expect(page.url()).toBe(baseURL + '/anchor#last-anchor-2'); const scrollY = /** @type {number} */ (await page.evaluate(() => scrollY)); expect(scrollY).toEqual(originalScrollY); - await page.goBack(); - expect(page.url()).toBe(baseURL + '/anchor'); - expect(scrollY).toEqual(0); + // TODO: fix this. it is failing due to duplicate history entries + // https://github.com/sveltejs/kit/issues/3636 + // await page.goBack(); + // expect(page.url()).toBe(baseURL + '/anchor'); + // expect(scrollY).toEqual(0); }); test('url-supplied anchor is ignored with onMount() scrolling on direct page load', async ({ diff --git a/packages/kit/test/utils.js b/packages/kit/test/utils.js index ae116dc5f139..a4fda1c72da1 100644 --- a/packages/kit/test/utils.js +++ b/packages/kit/test/utils.js @@ -127,6 +127,7 @@ export const test = base.extend({ setTimeout(() => { reject(new Error('Timed out')); }, 5000); + addEventListener('sveltekit:start', () => { fulfil(); }); From 39f34fddcbafd4913a8cc8e712d887cc7fe9fde2 Mon Sep 17 00:00:00 2001 From: Mihael Date: Tue, 15 Feb 2022 13:23:59 +0100 Subject: [PATCH 50/63] cleanup and should fix ctrl shift t issue --- packages/kit/src/runtime/client/router.js | 43 ++++++++++++++--------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index e964dc4db802..e08e511cdc7b 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -55,13 +55,27 @@ export class Router { // make it possible to reset focus document.body.setAttribute('tabindex', '-1'); + history.scrollRestoration = 'manual'; + // keeping track of the history index in order to prevent popstate navigation events if needed /** @type {number} */ - this.current_history_index = history.state?.['sveltekit:index'] ?? 0; + this.current_history_index = Number( + sessionStorage.getItem('sveltekit:index') ?? history.state?.['sveltekit:index'] ?? 0 + ); - if (this.current_history_index === 0) { + if (!history.state || history.state['sveltekit:index'] == undefined) { // create initial history entry, so we can return here - history.replaceState({ ...history.state, 'sveltekit:index': 0 }, '', location.href); + history.replaceState( + { ...(history.state || {}), 'sveltekit:index': this.current_history_index }, + '', + location.href + ); + } else if (performance.navigation?.type === 2) { + // performance.navigation.type is deprecated, but it's the only way to safely detect return to page + // and even if it's not supported, the only thing not working is ability to restore scroll state + // when returned to the page from different origin + const scroll = this.#get_scroll_position(this.current_history_index); + if (scroll) scrollTo(scroll.x, scroll.y); } this.callbacks = { @@ -73,7 +87,7 @@ export class Router { }; } - save_scroll_position() { + #save_scroll_position() { sessionStorage.setItem( `sveltekit:scroll:${this.current_history_index}`, JSON.stringify(scroll_state()) @@ -84,10 +98,8 @@ export class Router { * @param {number} index * @returns {{x: number, y: number} | null} */ - get_scroll_position(index) { - if (index == undefined) { - return null; - } + #get_scroll_position(index) { + if (index == undefined) return null; const scroll = sessionStorage.getItem(`sveltekit:scroll:${index}`); if (scroll) { return JSON.parse(scroll); @@ -97,10 +109,6 @@ export class Router { } init_listeners() { - if ('scrollRestoration' in history) { - history.scrollRestoration = 'manual'; - } - // Adopted from Nuxt.js // Reset scrollRestoration to auto when leaving page, allowing page reload // and back-navigation from other pages to use the browser to restore the @@ -120,11 +128,12 @@ export class Router { e.preventDefault(); e.returnValue = ''; } else { - history.scrollRestoration = 'auto'; + sessionStorage.setItem('sveltekit:index', String(this.current_history_index)); } }); // Setting scrollRestoration to manual again when returning to this page. + // We can probably safely delete this because constructor executes when returned to the page, but we need to test this to be sure, letting it stay for now. addEventListener('load', () => { history.scrollRestoration = 'manual'; }); @@ -195,7 +204,7 @@ export class Router { // Removing the hash does a full page navigation in the browser, so make sure a hash is present const [base, hash] = url.href.split('#'); if (hash !== undefined && base === location.href.split('#')[0]) { - this.save_scroll_position(); + this.#save_scroll_position(); // Call `replaceState` in timeout because this event will add pushState automatically and we need to store our own history state // Timeout is being used to delay replaceState till next tick @@ -233,11 +242,11 @@ export class Router { // with history.go, which means we end up back here, hence this check if (event.state['sveltekit:index'] === this.current_history_index) return; - this.save_scroll_position(); + this.#save_scroll_position(); this._navigate({ url: new URL(location.href), - scroll: this.get_scroll_position(event.state['sveltekit:index']), + scroll: this.#get_scroll_position(event.state['sveltekit:index']), keepfocus: false, chain: [], details: null, @@ -400,7 +409,7 @@ export class Router { }); } - this.save_scroll_position(); + this.#save_scroll_position(); accepted(); if (!this.navigating) { From c88c9b49e2a75b59b725bfce564f8d0de81b4db2 Mon Sep 17 00:00:00 2001 From: Mihael Date: Tue, 15 Feb 2022 13:28:14 +0100 Subject: [PATCH 51/63] fix --- packages/kit/src/runtime/client/router.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index e08e511cdc7b..9e702da531ba 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -128,6 +128,7 @@ export class Router { e.preventDefault(); e.returnValue = ''; } else { + this.#save_scroll_position(); sessionStorage.setItem('sveltekit:index', String(this.current_history_index)); } }); From d8527f37a39d8ca5c445037468c9a958383f532d Mon Sep 17 00:00:00 2001 From: Mihael Date: Tue, 15 Feb 2022 13:51:49 +0100 Subject: [PATCH 52/63] using sessionStorage instead of deprecated window.performance method --- packages/kit/src/runtime/client/router.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index 9e702da531ba..d6c609ea9814 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -70,10 +70,9 @@ export class Router { '', location.href ); - } else if (performance.navigation?.type === 2) { - // performance.navigation.type is deprecated, but it's the only way to safely detect return to page - // and even if it's not supported, the only thing not working is ability to restore scroll state - // when returned to the page from different origin + } else if (sessionStorage.getItem('sveltekit:left_page') === 'true') { + // detects return to the page + sessionStorage.removeItem('sveltekit:left_page'); const scroll = this.#get_scroll_position(this.current_history_index); if (scroll) scrollTo(scroll.x, scroll.y); } @@ -130,6 +129,7 @@ export class Router { } else { this.#save_scroll_position(); sessionStorage.setItem('sveltekit:index', String(this.current_history_index)); + sessionStorage.setItem('sveltekit:left_page', 'true'); } }); From 646dd69a25f306fb67f0c31c516ac63f5904a3da Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Tue, 15 Feb 2022 09:22:41 -0800 Subject: [PATCH 53/63] Revert "revert test" This reverts commit 61606c23a3e587faa58d923b01da72856a868383. --- packages/kit/test/apps/basics/test/test.js | 8 +++----- packages/kit/test/utils.js | 1 - 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index 729ac83dc188..c3e31270b8d5 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -214,11 +214,9 @@ test.describe('Scrolling', () => { expect(page.url()).toBe(baseURL + '/anchor#last-anchor-2'); const scrollY = /** @type {number} */ (await page.evaluate(() => scrollY)); expect(scrollY).toEqual(originalScrollY); - // TODO: fix this. it is failing due to duplicate history entries - // https://github.com/sveltejs/kit/issues/3636 - // await page.goBack(); - // expect(page.url()).toBe(baseURL + '/anchor'); - // expect(scrollY).toEqual(0); + await page.goBack(); + expect(page.url()).toBe(baseURL + '/anchor'); + expect(scrollY).toEqual(0); }); test('url-supplied anchor is ignored with onMount() scrolling on direct page load', async ({ diff --git a/packages/kit/test/utils.js b/packages/kit/test/utils.js index a4fda1c72da1..ae116dc5f139 100644 --- a/packages/kit/test/utils.js +++ b/packages/kit/test/utils.js @@ -127,7 +127,6 @@ export const test = base.extend({ setTimeout(() => { reject(new Error('Timed out')); }, 5000); - addEventListener('sveltekit:start', () => { fulfil(); }); From c12c44e7f270d3ba151b6972cce32e218c885d98 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Tue, 15 Feb 2022 09:24:34 -0800 Subject: [PATCH 54/63] Update packages/kit/src/runtime/client/router.js Co-authored-by: Rich Harris --- packages/kit/src/runtime/client/router.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index d6c609ea9814..f94352e20d14 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -63,7 +63,7 @@ export class Router { sessionStorage.getItem('sveltekit:index') ?? history.state?.['sveltekit:index'] ?? 0 ); - if (!history.state || history.state['sveltekit:index'] == undefined) { + if (history.state?.['sveltekit:index'] === undefined) { // create initial history entry, so we can return here history.replaceState( { ...(history.state || {}), 'sveltekit:index': this.current_history_index }, From 296a652aad27ed5dd9be01fa051cde442ddc7e14 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mihael=20Muti=C4=87?= <35368241+PH4NTOMiki@users.noreply.github.com> Date: Tue, 15 Feb 2022 18:27:55 +0100 Subject: [PATCH 55/63] Update packages/kit/src/runtime/client/router.js Co-authored-by: Rich Harris --- packages/kit/src/runtime/client/router.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index f94352e20d14..cb4fc41b984e 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -66,7 +66,7 @@ export class Router { if (history.state?.['sveltekit:index'] === undefined) { // create initial history entry, so we can return here history.replaceState( - { ...(history.state || {}), 'sveltekit:index': this.current_history_index }, + { ...history.state, 'sveltekit:index': this.current_history_index }, '', location.href ); From b1410096a73d839b8fca10e38d3b2de10559dc48 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Tue, 15 Feb 2022 09:29:52 -0800 Subject: [PATCH 56/63] Update packages/kit/src/runtime/client/router.js Co-authored-by: Rich Harris --- packages/kit/src/runtime/client/router.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index cb4fc41b984e..4abb4a3baf46 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -211,7 +211,7 @@ export class Router { // Timeout is being used to delay replaceState till next tick setTimeout(() => { history.replaceState( - { ...(history.state || {}), 'sveltekit:index': ++this.current_history_index }, + { ...history.state, 'sveltekit:index': ++this.current_history_index }, '', location.href ); From 037d24f2f14f5a71a92f4fc380bedfb9a7478990 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Tue, 15 Feb 2022 09:41:13 -0800 Subject: [PATCH 57/63] go back to previous version of test --- packages/kit/test/apps/basics/test/test.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index c3e31270b8d5..aad33d857e44 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -212,11 +212,10 @@ test.describe('Scrolling', () => { await clicknav('#routing-page'); await back(); expect(page.url()).toBe(baseURL + '/anchor#last-anchor-2'); - const scrollY = /** @type {number} */ (await page.evaluate(() => scrollY)); - expect(scrollY).toEqual(originalScrollY); + expect(await page.evaluate(() => scrollY)).toEqual(originalScrollY); await page.goBack(); expect(page.url()).toBe(baseURL + '/anchor'); - expect(scrollY).toEqual(0); + expect(await page.evaluate(() => scrollY)).toEqual(0); }); test('url-supplied anchor is ignored with onMount() scrolling on direct page load', async ({ From f8f3db1088ac6ff201796887312806f1ef70fa7b Mon Sep 17 00:00:00 2001 From: Mihael Date: Tue, 15 Feb 2022 19:18:30 +0100 Subject: [PATCH 58/63] trying fix for FireFox --- packages/kit/src/runtime/client/router.js | 24 +++++++++-------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index 4abb4a3baf46..782a03f75404 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -70,9 +70,9 @@ export class Router { '', location.href ); - } else if (sessionStorage.getItem('sveltekit:left_page') === 'true') { + } else if (sessionStorage.getItem('sveltekit:exited') === 'true') { // detects return to the page - sessionStorage.removeItem('sveltekit:left_page'); + sessionStorage.removeItem('sveltekit:exited'); const scroll = this.#get_scroll_position(this.current_history_index); if (scroll) scrollTo(scroll.x, scroll.y); } @@ -108,10 +108,6 @@ export class Router { } init_listeners() { - // Adopted from Nuxt.js - // Reset scrollRestoration to auto when leaving page, allowing page reload - // and back-navigation from other pages to use the browser to restore the - // scrolling position. addEventListener('beforeunload', (e) => { let should_block = false; @@ -129,7 +125,7 @@ export class Router { } else { this.#save_scroll_position(); sessionStorage.setItem('sveltekit:index', String(this.current_history_index)); - sessionStorage.setItem('sveltekit:left_page', 'true'); + sessionStorage.setItem('sveltekit:exited', 'true'); } }); @@ -206,20 +202,18 @@ export class Router { const [base, hash] = url.href.split('#'); if (hash !== undefined && base === location.href.split('#')[0]) { this.#save_scroll_position(); + + event.preventDefault(); - // Call `replaceState` in timeout because this event will add pushState automatically and we need to store our own history state - // Timeout is being used to delay replaceState till next tick - setTimeout(() => { + this.renderer.stores.page.subscribe((page) => { + this.renderer.stores.page.set({ ...page, url: new URL(url.href) }); + location.hash = url.hash; history.replaceState( { ...history.state, 'sveltekit:index': ++this.current_history_index }, '', location.href ); - }); - const info = this.parse(url); - if (info) { - return this.renderer.update(info, [], false); - } + })(); return; } From 427cc50e132203430bed0486fdd5c1953635346d Mon Sep 17 00:00:00 2001 From: Mihael Date: Tue, 15 Feb 2022 19:18:56 +0100 Subject: [PATCH 59/63] lint FF fix --- packages/kit/src/runtime/client/router.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index 782a03f75404..fcc4bb61a1f3 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -202,7 +202,7 @@ export class Router { const [base, hash] = url.href.split('#'); if (hash !== undefined && base === location.href.split('#')[0]) { this.#save_scroll_position(); - + event.preventDefault(); this.renderer.stores.page.subscribe((page) => { From 4fc8c3d49c9a2e15297df3000f4745876ff3e718 Mon Sep 17 00:00:00 2001 From: Mihael Date: Tue, 15 Feb 2022 19:31:24 +0100 Subject: [PATCH 60/63] different FF fix --- packages/kit/src/runtime/client/router.js | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index fcc4bb61a1f3..d53bd5e2411f 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -203,17 +203,19 @@ export class Router { if (hash !== undefined && base === location.href.split('#')[0]) { this.#save_scroll_position(); - event.preventDefault(); - - this.renderer.stores.page.subscribe((page) => { - this.renderer.stores.page.set({ ...page, url: new URL(url.href) }); - location.hash = url.hash; + // Call `replaceState` in Promise.resolve because this event will add pushState automatically and we need to store our own history state + // Promise.resolve is being used to delay replaceState till next tick + Promise.resolve(() => { history.replaceState( { ...history.state, 'sveltekit:index': ++this.current_history_index }, '', location.href ); - })(); + }); + const info = this.parse(url); + if (info) { + return this.renderer.update(info, [], false); + } return; } From 60dad678c965b30577f888a6d83d639f39e7775e Mon Sep 17 00:00:00 2001 From: Mihael Date: Tue, 15 Feb 2022 19:40:59 +0100 Subject: [PATCH 61/63] fix2 --- packages/kit/src/runtime/client/router.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index d53bd5e2411f..1ada7cab7c50 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -206,11 +206,13 @@ export class Router { // Call `replaceState` in Promise.resolve because this event will add pushState automatically and we need to store our own history state // Promise.resolve is being used to delay replaceState till next tick Promise.resolve(() => { - history.replaceState( - { ...history.state, 'sveltekit:index': ++this.current_history_index }, - '', - location.href - ); + setTimeout(() => { + history.replaceState( + { ...history.state, 'sveltekit:index': ++this.current_history_index }, + '', + location.href + ); + }); }); const info = this.parse(url); if (info) { From ee51709f802b36999ba859cfca8d821dee745ceb Mon Sep 17 00:00:00 2001 From: Mihael Date: Tue, 15 Feb 2022 19:50:48 +0100 Subject: [PATCH 62/63] FF fix --- packages/kit/src/runtime/client/router.js | 26 +++++++++++++---------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index 1ada7cab7c50..87087d877848 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -77,6 +77,9 @@ export class Router { if (scroll) scrollTo(scroll.x, scroll.y); } + /** @type {boolean} */ + this.hash_navigating = false; + this.callbacks = { /** @type {Array<({ from, to, cancel }: { from: URL, to: URL | null, cancel: () => void }) => void>} */ before_navigate: [], @@ -202,18 +205,8 @@ export class Router { const [base, hash] = url.href.split('#'); if (hash !== undefined && base === location.href.split('#')[0]) { this.#save_scroll_position(); + this.hash_navigating = true; - // Call `replaceState` in Promise.resolve because this event will add pushState automatically and we need to store our own history state - // Promise.resolve is being used to delay replaceState till next tick - Promise.resolve(() => { - setTimeout(() => { - history.replaceState( - { ...history.state, 'sveltekit:index': ++this.current_history_index }, - '', - location.href - ); - }); - }); const info = this.parse(url); if (info) { return this.renderer.update(info, [], false); @@ -259,6 +252,17 @@ export class Router { }); } }); + + addEventListener('hashchange', () => { + if (this.hash_navigating) { + this.hash_navigating = false; + history.replaceState( + { ...history.state, 'sveltekit:index': ++this.current_history_index }, + '', + location.href + ); + } + }); } /** From 8d44831018172bbbdf13de2dc61b68ce6fda7815 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mihael=20Muti=C4=87?= <35368241+PH4NTOMiki@users.noreply.github.com> Date: Tue, 15 Feb 2022 19:58:56 +0100 Subject: [PATCH 63/63] removed unneeded code --- packages/kit/src/runtime/client/router.js | 6 ------ 1 file changed, 6 deletions(-) diff --git a/packages/kit/src/runtime/client/router.js b/packages/kit/src/runtime/client/router.js index 87087d877848..e61d375db689 100644 --- a/packages/kit/src/runtime/client/router.js +++ b/packages/kit/src/runtime/client/router.js @@ -132,12 +132,6 @@ export class Router { } }); - // Setting scrollRestoration to manual again when returning to this page. - // We can probably safely delete this because constructor executes when returned to the page, but we need to test this to be sure, letting it stay for now. - addEventListener('load', () => { - history.scrollRestoration = 'manual'; - }); - /** @param {Event} event */ const trigger_prefetch = (event) => { const a = find_anchor(event);