From 1c99087490ff6ad1b1750450df4f29bd4c65a745 Mon Sep 17 00:00:00 2001 From: Matthijs Kralt Date: Mon, 4 Mar 2024 14:15:36 +0800 Subject: [PATCH 1/4] fix: Wait for previous navigation done before navigating to param with new updated value --- src/lib/sveltekit-search-params.ts | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/src/lib/sveltekit-search-params.ts b/src/lib/sveltekit-search-params.ts index e3e7bf1..5a18c3a 100644 --- a/src/lib/sveltekit-search-params.ts +++ b/src/lib/sveltekit-search-params.ts @@ -2,7 +2,7 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ import { browser, building } from '$app/environment'; import { goto } from '$app/navigation'; -import { page as page_store } from '$app/stores'; +import { navigating, page as page_store } from '$app/stores'; import type { Page } from '@sveltejs/kit'; import { derived, @@ -326,8 +326,28 @@ export function queryParam( const override = writable(null); let firstTime = true; let currentValue: T | null; + + let isNavigating = false; + if (browser) { + navigating.subscribe((nav) => { + isNavigating = nav?.type === 'goto'; + }); + } + function _set(value: T | null, changeImmediately?: boolean) { if (!browser) return; + + // Wait for previous navigation to be finished before updating again + if (isNavigating) { + const unsubscribe = navigating.subscribe((nav) => { + if (nav?.type !== 'goto') { + _set(value, changeImmediately); + unsubscribe(); + } + }); + return; + } + firstTime = false; const hash = window.location.hash; const toBatch = (query: URLSearchParams) => { From 8c31407f1eadb14b7a1b7601897cd8eac250d742 Mon Sep 17 00:00:00 2001 From: Matthijs Kralt Date: Thu, 21 Mar 2024 13:05:26 +0800 Subject: [PATCH 2/4] fix: Link unsubbing of store with unsubbing of isNavigating --- src/lib/sveltekit-search-params.ts | 32 ++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/src/lib/sveltekit-search-params.ts b/src/lib/sveltekit-search-params.ts index 5a18c3a..0945345 100644 --- a/src/lib/sveltekit-search-params.ts +++ b/src/lib/sveltekit-search-params.ts @@ -11,6 +11,8 @@ import { type Updater, type Writable, type Readable, + type Subscriber, + type Unsubscriber, readable, } from 'svelte/store'; import { @@ -328,11 +330,6 @@ export function queryParam( let currentValue: T | null; let isNavigating = false; - if (browser) { - navigating.subscribe((nav) => { - isNavigating = nav?.type === 'goto'; - }); - } function _set(value: T | null, changeImmediately?: boolean) { if (!browser) return; @@ -396,10 +393,10 @@ export function queryParam( }); } - const { subscribe } = derived<[typeof page, typeof override], T | null>( + const store = derived<[typeof page, typeof override], T | null>( [page, override], ([$page, $override], set) => { - if ($override) { + if ($override != undefined) { if (isComplexEqual(currentValue, $override, equalityFn)) { return; } @@ -429,10 +426,29 @@ export function queryParam( set(newValue) { _set(newValue); }, - subscribe, update: (updater: Updater) => { const newValue = updater(currentValue); _set(newValue); }, + subscribe( + run: Subscriber, + invalidate?: (value?: T | null) => void, + ): Unsubscriber { + // Subscribe to the derived store + const storeUnsubscribe = store.subscribe(run, invalidate); + // Subscribe to isNavigating + let unsubscribeNavigating: () => void; + if (browser) { + unsubscribeNavigating = navigating.subscribe((nav) => { + isNavigating = nav?.type === 'goto'; + }); + } + return () => { + storeUnsubscribe(); + if (unsubscribeNavigating) { + unsubscribeNavigating(); + } + }; + }, }; } From 4c7d474d4657f5f2eb034bd7ab4021737fc47fe8 Mon Sep 17 00:00:00 2001 From: Matthijs Kralt Date: Thu, 21 Mar 2024 14:09:11 +0800 Subject: [PATCH 3/4] test: Add test for the param navigation bug --- .../routes/paramOnNavigateBug/+page.svelte | 15 ++++++++++++++ tests/index.test.ts | 20 +++++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 playground/src/routes/paramOnNavigateBug/+page.svelte diff --git a/playground/src/routes/paramOnNavigateBug/+page.svelte b/playground/src/routes/paramOnNavigateBug/+page.svelte new file mode 100644 index 0000000..0ce401f --- /dev/null +++ b/playground/src/routes/paramOnNavigateBug/+page.svelte @@ -0,0 +1,15 @@ + + + diff --git a/tests/index.test.ts b/tests/index.test.ts index 6cd6bd4..6dcdf3b 100644 --- a/tests/index.test.ts +++ b/tests/index.test.ts @@ -133,6 +133,26 @@ test.describe('queryParam', () => { expect(url.hash).toBe('#test-hash'); }); + test('changing multiple parameters due to subscribe updates params accordingly', async ({ + page, + }) => { + await page.goto('/paramOnNavigateBug'); + const btn = page.getByTestId('update-params'); + await btn.click(); + + // expect(url.searchParams.get('param1')).toBe(''); + + await page.waitForURL( + (url) => { + return ( + url.searchParams.get('param1') === 'updated param1' && + url.searchParams.get('param2') === 'updated param2' + ); + }, + { timeout: 1000 }, + ); + }); + test("changing two parameters in the same function doesn't negate", async ({ page, }) => { From 3d6e5fe0667e6a18875653311a06537ae81c0c32 Mon Sep 17 00:00:00 2001 From: Matthijs Kralt Date: Thu, 21 Mar 2024 14:50:13 +0800 Subject: [PATCH 4/4] fix: add ondestroy unsub for param subscribing in playground case --- playground/src/routes/paramOnNavigateBug/+page.svelte | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/playground/src/routes/paramOnNavigateBug/+page.svelte b/playground/src/routes/paramOnNavigateBug/+page.svelte index 0ce401f..1c00224 100644 --- a/playground/src/routes/paramOnNavigateBug/+page.svelte +++ b/playground/src/routes/paramOnNavigateBug/+page.svelte @@ -1,12 +1,17 @@