-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
False positive pushstate warnings #11671
Comments
I get this warning as soon as I do use As much as I appreciate the work that went into I'd much prefer to be able to use |
SvelteKit is not overriding a browser API - we only patch the |
Getting a false positive as well, also with any |
Also receiving a false positive when calling goto. Given the warnings we're receiving, it would be helpful if the documentation was also updated to include multiple examples of replaceState / pushState. Page state as a required parameter for replaceState is confusing and inconsistent with the original History API. We'd ideally do a drop-in replacement for history.replaceState when changing the query parameters via shallow routing, but we cannot do so currently and instead have to wrestle with the unclear example usage of replaceState. StackOverflow currently recommends using goto but as mentioned in #11465, this behavior is also inconsistent. If replaceState and pushState could mirror the History API as closely as possible, thorough documentation of example usage wouldn't be necessary. Thanks so much for the help! |
fixes #11671 The warning currently incorrectly fires when we use history.pushState() internally such as clicking on a link. You can test this in a stackblitz starter project. This is because one of the early exit conditions in the warning method is failing incorrectly. import.meta.url returns the client.js file with a query parameter when loading it from node_modules. This causes the substring search for the module filename in the error stack to always return false (because of the query params that are not present in the error stack) The solution is to simply return the URL without the query parameters if any.
I still have the issue, but I'm not sure what I can do since it doesn't look like file I own. Stack is refering to: Promise.all([
import("/@fs/.../node_modules/.pnpm/@sveltejs+kit@2.5.2_@sveltejs+vite-plugin-svelte@3.0.2_svelte@4.2.7_vite@5.1.4/node_modules/@sveltejs/kit/src/runtime/client/entry.js"),
import("/@fs/.../apps/cadb/.svelte-kit/generated/client/app.js")
]).then(([kit, app]) => {
kit.start(app, element);
}); At first, I was thinking that it was due to paoloricciuti/sveltekit-search-params#66 but it doens't look like. |
Can you open a new issue with a reproduction? |
I tried and didn't manage to reproduce in a small app :/ I'll try to add more time into finding the root cause, but can't right now. |
Getting this when using Open Telemetry tracing on client side. The library is using push state, none of my internal code calls push state. |
Hi!
My code roughly looks like this: import { replaceState } from "$app/navigation"
export const replaceStateWithQuery = (values: Record<string, string | string[] | undefined>) => {
const url = new URL(window.location.toString())
for (const [key, value] of Object.entries(values)) {
if (value && Array.isArray(value)) {
value.forEach(val => {
url.searchParams.append(key, val)
})
} else if (value) {
url.searchParams.set(key, value)
} else {
url.searchParams.delete(key)
}
}
replaceState(url, {})
} Then I run |
I'm also getting this warning in my svelte app while using |
I'm also using sentry, you think it can be the root cause ? |
@fromaline Hey, have the same issue now.. Do you got any fix for it? |
@lecramr, nope |
@fromaline where are you doing the replaceState? I was doing it in the onMount wrapping it in a setTimeout((), 50) fixed it for me :) |
I'm not using
history
as suggested by the warning. I'm usinggoto
in a couple of places, from$app/navigation
. I haven't changed anything. The warning is not present withv2.3.3
.From a quick test, commenting out all the
goto
calls still resulted in the warning.Originally posted by @lukasmalkmus in #11657 (comment)
I think this happens because when a library like Google Analytics monkey patches pushstate it will be next in the call stack, resulting in the false positive. I'm not sure how to solve this other than checking the whole stack for an occurrence of the client file, which may result in false negatives.
The text was updated successfully, but these errors were encountered: