-
Notifications
You must be signed in to change notification settings - Fork 926
Browser Extension Check for Analytics Module #3555
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
Conversation
…tion if under browser extension context
🦋 Changeset is good to goLatest commit: bbfe8a6 We got this. 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 |
packages/analytics/src/errors.ts
Outdated
'Cookies are not enabled in this browser environment. Analytics requires cookies to be enabled.' | ||
'Cookies are not enabled in this browser environment. Analytics requires cookies to be enabled.', | ||
[AnalyticsError.INVALID_ANALYTICS_CONTEXT]: | ||
'Analytics module is not supported in browser extensions environemnt.' |
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.
How about Firebase Analytics is not supported in browser extensions.
.changeset/spicy-masks-sort.md
Outdated
@@ -0,0 +1,5 @@ | |||
--- | |||
"@firebase/analytics": minor |
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.
I think this is a patch change since we are not changing the API surface.
…m/firebase/firebase-js-sdk into browser-extension-check-analytics
Binary Size ReportAffected SDKs
Test Logs |
.changeset/spicy-masks-sort.md
Outdated
"@firebase/analytics": patch | ||
--- | ||
|
||
Browser Extension Check for Analytics Module |
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.
Can you make it a bit more specific? Something like: analytics.isSupport() will now return false for extension environments
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.
Can you add firebase: patch
too, so we can generate changelog for firebase
?
"@firebase/analytics": patch | ||
--- | ||
|
||
Added Browser Extension check for Firebase Analytics. analytics.isSupported() will now return Promise<false> for extension environments. |
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.
Nit: backticks around analytics.isSupported()
and Promise<false>
will make it look nicer in markdown.
added browser extension check to analytics module; prevent initialization if not in a browser context.