Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wait for previous navigation done before navigating to param with new updated value #73

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions playground/src/routes/paramOnNavigateBug/+page.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<script lang="ts">
import { onDestroy } from 'svelte';
import { queryParam, ssp } from 'sveltekit-search-params';

const param1 = queryParam('param1', ssp.string());
const param2 = queryParam('param2', ssp.string());

const unsubscribe = param1.subscribe(() => {
param2.set('updated param2');
});

onDestroy(() => {
unsubscribe();
});
</script>

<button
data-testid="update-params"
on:click={() => ($param1 = 'updated param1')}>Update params</button
>
44 changes: 40 additions & 4 deletions src/lib/sveltekit-search-params.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -11,6 +11,8 @@ import {
type Updater,
type Writable,
type Readable,
type Subscriber,
type Unsubscriber,
readable,
} from 'svelte/store';
import {
Expand Down Expand Up @@ -326,8 +328,23 @@ export function queryParam<T = string>(
const override = writable<T | null>(null);
let firstTime = true;
let currentValue: T | null;

let isNavigating = false;

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) => {
Expand Down Expand Up @@ -376,10 +393,10 @@ export function queryParam<T = string>(
});
}

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;
}
Expand Down Expand Up @@ -409,10 +426,29 @@ export function queryParam<T = string>(
set(newValue) {
_set(newValue);
},
subscribe,
update: (updater: Updater<T | null>) => {
const newValue = updater(currentValue);
_set(newValue);
},
subscribe(
run: Subscriber<T | null>,
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();
}
};
},
};
}
20 changes: 20 additions & 0 deletions tests/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}) => {
Expand Down
Loading