firebase-js-sdk icon indicating copy to clipboard operation
firebase-js-sdk copied to clipboard

Not possible to use setUserId to unset user id using firebase/analytics

Open buren opened this issue 2 years ago • 2 comments

Describe your environment

  • Operating System version: OSX
  • Browser version: Chrome 104
  • Firebase SDK version: v9.8.2
  • Firebase Product: Analytics (auth, database, storage, etc)

Describe the problem

When a user logs out it is currently not possible to unset the user id.

Steps to reproduce:

In code

  1. Initialize a firebase app
  2. Initialize an analytics instance
  3. Try to set the user id to null using setUserId(analytics, null)
  4. Type error

Relevant Code:

To reproduce

import { setUserId } from 'firebase/analytics'

// initialize app 
const analytics = getAnalytics(app)
setUserId(analytics, null)
// Argument of type 'null' is not assignable to parameter of type 'string'.ts

The function signature differs from other places, for example https://github.com/firebase/firebase-js-sdk/blob/1d3a34d7da5bf3c267d014efb587e03c46ff3064/packages/analytics/src/functions.ts#L96 Here it seems perfectly possible to set the user id to null.

Here null is not allowed

  • https://github.com/firebase/firebase-js-sdk/blob/11d37eddc47d8f0ca07915135af183bc10a06814/packages/analytics-types/index.d.ts#L465
  • https://github.com/firebase/firebase-js-sdk/blob/1d3a34d7da5bf3c267d014efb587e03c46ff3064/packages/analytics/src/api.ts#L180

Here in the documentation for Firebase analytics for android it even states explicitly that null should be allowed for compliant implementations. Same thing here for iOS

Here's a similar issue for Firebase React Native https://github.com/invertase/react-native-firebase/issues/4931 and here is another issue in the same repository that says that the documentation says that it's allowed but that the implementation actually doesn't set the userId to null - https://github.com/firebase/firebase-android-sdk/issues/3602.

buren avatar Sep 08 '22 12:09 buren

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

google-oss-bot avatar Sep 08 '22 12:09 google-oss-bot

So it seems like there's 2 issues:

  1. The param type is wrong. The internal implementation of this function does allow for string | null and the public-facing wrapper function should as well. We can fix this.

https://github.com/firebase/firebase-js-sdk/blob/87c90a78d613e22349f46bd72add64425882fc59/packages/analytics/src/functions.ts#L96

  1. Setting it to null may not actually do anything in Android (perhaps due to a bug) and you're wondering if it's the same on web. There's 2 ways to find out. The easiest way might be to just use a @ts-ignore on that line really quick to ignore the type check error and just see if it actually sets it. Another way is to set it by calling gtag() directly (gtag('config', YOUR_MEASUREMENT_ID, {'user_id': null}). (If you do this, it may not show up in your Firebase Analytics dashboard and you may have to check your Google Analytics dashboard for it.)

If I have time I can try it out, but if you want to speed things up, you can try both of those things and let me know what happens. They may possibly get different results because they're not exactly identical calls.

hsubox76 avatar Sep 08 '22 16:09 hsubox76

The fix has been released in v9.12.1.

dwyfrequency avatar Oct 18 '22 14:10 dwyfrequency