-
Notifications
You must be signed in to change notification settings - Fork 443
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(firefox): remove setTimeout from onLocationChange #511
fix(firefox): remove setTimeout from onLocationChange #511
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
This looks reasonable, I will try to dig into why setTimeout
is there. In the meantime could we have a test https://github.com/BuilderIO/partytown/tree/main/tests/platform which shows that the issue has been fixed?
can you help me create a proper test? I tried adding "firefox" to playwright but almost all the tests are failing. So I'm not sure on how to proceed and I prefer waiting for some help. The issue happens only in firefox, at first page change. The open bugs are at least these: |
HI @emish89 |
great, so you think can be merged? |
there is not tests for |
I investigated and |
@gioboa I was involved in the astro fix. If you see the code: https://github.com/withastro/astro/pull/9419/files you can see that the history APIs has been replaced. With these changes, from your point of view, partytown library is still working as expected? It was intended as a temporary fix until partytown solves the problems with firefox from what I've understood. |
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.
LGTM, but there is a failure.
@emish89 the failing test is a flaky one. Now we have a green line. |
What is it?
Description
I removed a setTimeout that was giving error on firefox after page change. There are more open issues for this.
Use cases and why
I expect that now firefox works