Skip to content
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

Browser Extension Check for Analytics Module #3555

Merged
merged 7 commits into from
Aug 5, 2020

Conversation

XuechunHou
Copy link
Contributor

added browser extension check to analytics module; prevent initialization if not in a browser context.

@changeset-bot
Copy link

changeset-bot bot commented Aug 3, 2020

🦋 Changeset is good to go

Latest 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

'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.'
Copy link
Contributor

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.

@@ -0,0 +1,5 @@
---
"@firebase/analytics": minor
Copy link
Contributor

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.

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Aug 3, 2020

Binary Size Report

Affected SDKs

  • @firebase/analytics

    Type Base (2fa0353) Head (8c29e63) Diff
    esm2017 9.66 kB 9.89 kB +229 B (+2.4%)
    main 10.9 kB 11.1 kB +225 B (+2.1%)
    module 10.6 kB 10.8 kB +234 B (+2.2%)
  • firebase

    Type Base (2fa0353) Head (8c29e63) Diff
    firebase-analytics.js 28.0 kB 28.3 kB +318 B (+1.1%)
    firebase.js 819 kB 819 kB +195 B (+0.0%)

Test Logs

"@firebase/analytics": patch
---

Browser Extension Check for Analytics Module
Copy link
Member

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

Copy link
Member

@Feiyang1 Feiyang1 Aug 3, 2020

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.
Copy link
Contributor

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.

@XuechunHou XuechunHou merged commit 2a0d254 into master Aug 5, 2020
@XuechunHou XuechunHou deleted the browser-extension-check-analytics branch August 5, 2020 16:06
@google-oss-bot google-oss-bot mentioned this pull request Aug 11, 2020
@firebase firebase locked and limited conversation to collaborators Sep 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants