-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
fix(web): infinite loop browser navigation crash admin settings page #14850
Conversation
@@ -35,7 +34,7 @@ | |||
searchParams.delete(queryParam); | |||
} | |||
|
|||
handlePromiseError(goto(`?${searchParams.toString()}`, { replaceState: true, noScroll: true, keepFocus: true })); | |||
replaceState(`?${searchParams.toString()}`, { replaceState: true, noScroll: true, keepFocus: true }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The state passed to replaceState
doesn't really make sense. I think goto
is still the right function to use and we should just be more careful about reactivity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Go to here seems to render again the page and cause this effect function to run again, which lead to an infinity loop. Any other suggestion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing reactivity from searchParams
is probably fine:
const searchParams = new URLSearchParams(page.url.searchParams);
$effect(() => {
if ($state.size > 0) {
searchParams.set(queryParam, [...$state].join(' '));
} else {
searchParams.delete(queryParam);
}
handlePromiseError(goto(`?${searchParams.toString()}`, { replaceState: true, noScroll: true, keepFocus: true }));
});
Or an example that includes reactivity for page.url.searchParams
:
let search = $derived.by(() => {
const searchParams = new URLSearchParams(page.url.searchParams);
if ($state.size > 0) {
searchParams.set(queryParam, [...$state].join(' '));
} else {
searchParams.delete(queryParam);
}
return searchParams.toString();
});
$effect(() => {
handlePromiseError(goto(`?${search}`, { replaceState: true, noScroll: true, keepFocus: true }));
});
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! That indeed fixes it!
No description provided.