signInWithPopup not working when used in library

IDrumsey commented 3 weeks ago

Operating System

Windows 11

Environment (if applicable)

Chrome 127

Firebase SDK Version

10.13.0

Firebase SDK Product(s)

Auth

Project Tooling

Next.js

Detailed Problem Description

I have a custom package I built with two functions

import { signInWithEmailAndPassword, signInWithPopup, OAuthProvider } from "firebase/auth";

export async function signinWithPassword(auth: Auth, email: string, password: string) {
    // Sign in with email and password

    return signInWithEmailAndPassword(auth, email, password)
}

export async function signinWithMicrosoft(auth: Auth, provider: OAuthProvider): ReturnType<typeof signInWithPopup> {
    // Sign in with Microsoft

    return signInWithPopup(auth, provider)
}

I created a testing application to test these functions. The signInWithPassword works flawlessly, but the signinWithMicrosoft doesn't work. Note that if I replace the call to signinWithMicrosoft(... in my client app with signInWithPopup from firebase/auth (which is exactly what I'm doing in the library), it starts working.

Here's the error I'm getting

FirebaseError: Firebase: Error (auth/argument-error).
    at createErrorInternal
    at _fail
    at _assertInstanceOf
    at signInWithPopup

Steps and code to reproduce issue

Create a basic npm package with those two functions
Create a basic next.js app and call those two functions. (You'll have to install firebase and get the auth object to pass... also will have to set up the provider.)

call each function and see how the sign in with password works, but the microsoft one doesn't.

google-oss-bot commented 3 weeks ago

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

dlarocque commented 3 weeks ago

Hey @IDrumsey, could you please share a code snippet that shows how you're calling signInWithMicrosoft?

It could be that the assertInstanceOf is failing because the provider parameter in signInWithMicrosoft is an OAuthProvider, when signInWithPopup defines it to be an AuthProvider. https://firebase.google.com/docs/reference/js/auth.md#signinwithpopup_770f816

I can't see the lines in the stack trace you added, but I believe the failing assertion is here, and my guess is that this condition is failing https://github.com/firebase/firebase-js-sdk/blob/main/packages/auth/src/core/util/assert.ts#L121

Could you try having signInWithMicrosoft accept an AuthProvider instead?

IDrumsey commented 3 weeks ago

@dlarocque, the stack locations do look correct where it's throwing. I replaced signInWithMicrosoft(auth: Auth, provider: OAuthProvider) with signInWithMicrosoft(auth: Auth, provider: AuthProvider). That did not work. Even switched to using Google as the provider instead of Microsoft since Microsoft uses OAuthProvider.

I'm just calling signInWithMicrosoft from a react component like the following.

import {provider, auth} from 'myfirebasefile'

const MyButton = (onClick: () => void) => {
  return <Box onClick={onClick}>my button</Box>
}

<MyButton onClick={() => signInWithMicrosoft(auth, provider)}

dlarocque commented 3 weeks ago

@IDrumsey, the following code works for me in a sample NextJS app.

"use client";
import { initializeApp } from "firebase/app";
import { getAuth, Auth, AuthProvider, signInWithPopup, UserCredential, GoogleAuthProvider } from "firebase/auth";

const firebaseConfig = {
/* */
}

const app = initializeApp(firebaseConfig);
const auth = getAuth(app);

async function signinWithMicrosoft(auth: Auth, provider: AuthProvider): Promise<UserCredential> {
  return signInWithPopup(auth, provider)
}

export default function Home() {
  return (
  <div>
    <button onClick={() => { signinWithMicrosoft(auth, new GoogleAuthProvider() ) }}>Click me</button>
  </div>
  );
}

Is there anything else you can share that might help me reproduce this issue? When I try to set the return type to ReturnType<typeof signInWithPopup> I get a TypeScript compiler error, because it's not a promise-compatible constructor value.

IDrumsey commented 3 weeks ago

@IDrumsey, the following code works for me in a sample NextJS app.
"use client";
import { initializeApp } from "firebase/app";
import { getAuth, Auth, AuthProvider, signInWithPopup, UserCredential, GoogleAuthProvider } from "firebase/auth";

const firebaseConfig = {
/* */
}

const app = initializeApp(firebaseConfig);
const auth = getAuth(app);

async function signinWithMicrosoft(auth: Auth, provider: AuthProvider): Promise<UserCredential> {
  return signInWithPopup(auth, provider)
}

export default function Home() {
  return (
  <div>
    <button onClick={() => { signinWithMicrosoft(auth, new GoogleAuthProvider() ) }}>Click me</button>
  </div>
  );
}
Is there anything else you can share that might help me reproduce this issue? When I try to set the return type to ReturnType<typeof signInWithPopup> I get a TypeScript compiler error, because it's not a promise-compatible constructor value.

Note this from above

Note that if I replace the call to signinWithMicrosoft(... in my client app with signInWithPopup from firebase/auth (which is exactly what I'm doing in the library), it starts working.

This is expected because you defined the signinWithMicrosoft function in your client application. I defined it in a separate npm package. Idk if it matters, but I'm using tsup to bundle the package. But again, idk if this would matter because the signInWithPassword function from my library IS working... just not the function that uses the popup function.

dlarocque commented 3 weeks ago

@IDrumsey, the following code works for me in a sample NextJS app.
"use client";
import { initializeApp } from "firebase/app";
import { getAuth, Auth, AuthProvider, signInWithPopup, UserCredential, GoogleAuthProvider } from "firebase/auth";

const firebaseConfig = {
/* */
}

const app = initializeApp(firebaseConfig);
const auth = getAuth(app);

async function signinWithMicrosoft(auth: Auth, provider: AuthProvider): Promise<UserCredential> {
  return signInWithPopup(auth, provider)
}

export default function Home() {
  return (
  <div>
    <button onClick={() => { signinWithMicrosoft(auth, new GoogleAuthProvider() ) }}>Click me</button>
  </div>
  );
}
Is there anything else you can share that might help me reproduce this issue? When I try to set the return type to ReturnType<typeof signInWithPopup> I get a TypeScript compiler error, because it's not a promise-compatible constructor value.
Note this from above

Note that if I replace the call to signinWithMicrosoft(... in my client app with signInWithPopup from firebase/auth (which is exactly what I'm doing in the library), it starts working.

This is expected because you defined the signinWithMicrosoft function in your client application. I defined it in a separate npm package. Idk if it matters, but I'm using tsup to bundle the package. But again, idk if this would matter because the signInWithPassword function from my library IS working... just not the function that uses the popup function.

If signInWithPopup works as expected when called from your Next.js client, but not when it's called in your bundle, then it's an issue with how you're bundling your library.

To help me identify if this is a bug and fix it, I need to be able to reproduce it. Could you provide a minimal codebase that successfully reproduces the issue?

IDrumsey commented 3 weeks ago

@dlarocque yes I will work on that. I forgot I did make a recreation of the package, but without typescript (so no bundling required) and still got the error. let me get you the files.

IDrumsey commented 3 weeks ago

@dlarocque Here's the basic setup. Includes the package files and the app files. Steps you'll have to take.

go into package directory and run npm i
run npm link from package directory
go to app directory and npm i, npm i firebase, npm link package, and fill in the firebase.ts file info
spin up the app npm run dev.
do the sign in with password. Notice how it doesn't throw an error, but sign in with microsoft does.

I made the problem even clearer by just importing the firebase functions in the package and exporting them. No proxy functions at all.

out.zip

dlarocque commented 3 weeks ago

@dlarocque Here's the basic setup. Includes the package files and the app files. Steps you'll have to take.

go into package directory and run npm i

run npm link from package directory

go to app directory and npm i, npm i firebase, npm link package, and fill in the firebase.ts file info

spin up the app npm run dev.

do the sign in with password. Notice how it doesn't throw an error, but sign in with microsoft does.

I made the problem even clearer by just importing the firebase functions in the package and exporting them. No proxy functions at all.

out.zip

Thanks for sharing this- very helpful!

So, here's the function that's causing trouble:

function _assertInstanceOf(auth, object, instance) {
    const constructorInstance = instance;
    if (!(object instanceof constructorInstance)) { // This is true!
        if (constructorInstance.name !== object.constructor.name) {
            _fail(auth, "argument-error" /* AuthErrorCode.ARGUMENT_ERROR */);
        }
        throw _errorWithCustomMessage(auth, "argument-error" /* AuthErrorCode.ARGUMENT_ERROR */, `Type of ${object.constructor.name} does not match expected instance.` +
            `Did you pass a reference from a different Auth SDK?`);
    }
}

When using signInWithPopup imported from firebase/auth in the client, object instanceof constructorInstance is true because object is a GoogleAuthProvider, which is in fact an instance of FederatedAuthProvider. This is the correct behaviour. But, when using signInWithPopup imported from your library, the GoogleAuthProvider passed is from the client's firebase package. So, even though it's the "same" object, since it's coming from an entirely different firebase package, it's not an actual instance of FederatedAuthProvider from your library's firebase package.

For the instanceof check to be true, the provider needs to come from the same firebase package as `signInWithPopup. I got it working by exporting the provider from the package and then imported that from the client:

'use client'

import {signInWithEmailAndPassword, signInWithPopup, GoogleAuthProvider } from 'package'
import { auth } from "@/firebase";
import { useState } from "react";

export default function Home() {

  const [email, setEmail] = useState('');
  const [password, setPassword] = useState('');

  return (
    <>
    <input type="email" placeholder="email" onChange={(e) => setEmail(e.target.value)} />
    <input type="password" placeholder="password" onChange={(e) => setPassword(e.target.value)} />
    <button onClick={() => signInWithEmailAndPassword(auth, email, password)}>sign in</button>
    <button onClick={() => signInWithPopup(auth, new GoogleAuthProvider())}>open popup</button>
    </>
  );
}

Then, in the library:

// package/index.js
export { signInWithEmailAndPassword, signInWithPopup, GoogleAuthProvider } from "firebase/auth";

dlarocque commented 3 weeks ago

Just wondering- the error message thrown says

Type of ${object.constructor.name} does not match expected instance.` +
            `Did you pass a reference from a different Auth SDK?`

Did you get this in your error message? I don't see it in the one you shared.

dlarocque commented 3 weeks ago

IDrumsey commented 3 weeks ago

Just wondering- the error message thrown says
Type of ${object.constructor.name} does not match expected instance.` +
            `Did you pass a reference from a different Auth SDK?`
Did you get this in your error message? I don't see it in the one you shared.

No.

IDrumsey commented 2 weeks ago

There is now a further issue. I got past the argument-error with your suggestion to export OAuthProvider from my library, but now when the popup opens i get The requested action is invalid. showing on the popup. localhost is an authorized domain on the firebase project already and I tested, like previously, without using the library and it starts working. Did you replicate my setup and it totally worked for you @dlarocque?

IDrumsey commented 2 weeks ago

There is now a further issue. I got past the argument-error with your suggestion to export OAuthProvider from my library, but now when the popup opens i get The requested action is invalid. showing on the popup. localhost is an authorized domain on the firebase project already and I tested, like previously, without using the library and it starts working. Did you replicate my setup and it totally worked for you @dlarocque?

Found a solution, although this is starting to feel jank. I had to export

export { initializeApp, getApps } from "firebase/app"
export { getAuth } from "firebase/auth"

from my library and use that when initializing my client side firebase client. I'm going to do some testing to see if every firebase utility needs to be proxied through my library to be usable. There has to be a reason why this is the case right?

dlarocque commented 2 weeks ago

There is now a further issue. I got past the argument-error with your suggestion to export OAuthProvider from my library, but now when the popup opens i get The requested action is invalid. showing on the popup. localhost is an authorized domain on the firebase project already and I tested, like previously, without using the library and it starts working. Did you replicate my setup and it totally worked for you @dlarocque?

Found a solution, although this is starting to feel jank. I had to export
export { initializeApp, getApps } from "firebase/app"
export { getAuth } from "firebase/auth"
from my library and use that when initializing my client side firebase client. I'm going to do some testing to see if every firebase utility needs to be proxied through my library to be usable. There has to be a reason why this is the case right?

instanceof checks (like the ones you ran into) failing is one of the reasons why this is the case. I would recommend against using two instances of the Firebase JS SDK together.

Since the original issue was resolved, I'm going to close this issue. If you find an issue in the Firebase JS SDK, please open another issue and share more information there.

IDrumsey commented 2 weeks ago

There is now a further issue. I got past the argument-error with your suggestion to export OAuthProvider from my library, but now when the popup opens i get The requested action is invalid. showing on the popup. localhost is an authorized domain on the firebase project already and I tested, like previously, without using the library and it starts working. Did you replicate my setup and it totally worked for you @dlarocque?

Found a solution, although this is starting to feel jank. I had to export
export { initializeApp, getApps } from "firebase/app"
export { getAuth } from "firebase/auth"
from my library and use that when initializing my client side firebase client. I'm going to do some testing to see if every firebase utility needs to be proxied through my library to be usable. There has to be a reason why this is the case right?
instanceof checks (like the ones you ran into) failing is one of the reasons why this is the case. I would recommend against using two instances of the Firebase JS SDK together.

Since the original issue was resolved, I'm going to close this issue. If you find an issue in the Firebase JS SDK, please open another issue and share more information there.

In my library I have firebase marked as a peerDependency. Doesn't that mean it should just use the instance of firebase from the client app, so 1 instance?

firebase / firebase-js-sdk