-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
ref(browser): Update supportsHistory
check & history usage
#14696
Conversation
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!
size-limit report 📦
|
fe76964
to
cc7dc32
Compare
cc7dc32
to
b034c35
Compare
❌ 3 Tests Failed:
View the top 3 failed tests by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
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.
Change LGTM! But I think the diff got messed up during the rebase.
b034c35
to
bbefd73
Compare
We had some elaborate check in there for
window.history
which is based on very old (~2016) issues, which should not be an issue anymore in versions we support today. So we can streamline this quite a bit!Additionally, I also rewrote code that used
window.onpushstate
to instead ofwindow.addEventListener('pushstate')
, plus also added code to ensure we do not emit history changes multiple times, which may be possible (?).This is on v9 branch.