-
Notifications
You must be signed in to change notification settings - Fork 907
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
isSupported fixes #5506
isSupported fixes #5506
Conversation
🦋 Changeset detectedLatest commit: b550e8d The changes in this PR will be included in the next version bump. This PR includes changesets to release 28 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Changeset File Check ✅
|
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 for performance. Thanks for making this change!
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.
Looks good, thanks for being so thorough!
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.
Looks like no new doc strings, lmk if I'm wrong about that. Thanks!
isSupported()
had bugs in messaging, performance, and analytics.!navigator
should betypeof navigator !== 'undefined
since it's a global. This led to runtime failures.areCookiesEnabled
testFurther, the indexedDb check was not working correctly in AppCheck due to checking for it on
self
(which isn't available in all environments). I addressed this and dry'd up ourtypeof indexedDb
checks to all useisIndexedDBAvailable()
.