Skip to content

Conversation

@ShaMan123
Copy link

startSignIn(auth: any, tenantInfo?: firebaseui.auth.SelectedTenantInfo):
getAuth(apiKey: string, tenantId: string | null): firebase.default.auth.Auth;
startSignIn(auth: firebase.default.auth.Auth, tenantInfo?: firebaseui.auth.SelectedTenantInfo):
Promise<any>; // tslint:disable-line
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the return type here?

@bojeil-google
Copy link
Contributor

bojeil-google commented Jun 29, 2021

Hey @ShaMan123, thanks for the contribution!
Can you get this to work without the default namespace in firebase.default?
The public types are defined as firebase.auth.UserCredential instead of firebase.default.auth.UserCredential.

@ShaMan123
Copy link
Author

ShaMan123 commented Jun 30, 2021

That's what I thought but the compiler throws an error without .default
It seems a firebase issue

@bojeil-google
Copy link
Contributor

That's what I thought but the compiler throws an error without .default
It seems a firebase issue

The problem is if we accept this, it could be a breaking change as it may not match the existing public types.

@ShaMan123
Copy link
Author

I disagree. Why would it be breaking? And why would it mismatch?

@bojeil-google
Copy link
Contributor

bojeil-google commented Jun 30, 2021

I disagree. Why would it be breaking? And why would it mismatch?

Hmm, I take that back as the underlying types should match, but we'd want to test this to confirm nothing unexpected surfaces. Regardless, can you open an issue in the firebase-js-sdk repo about the.default types?

@ShaMan123
Copy link
Author

Opened issue

@ShaMan123
Copy link
Author

ShaMan123 commented Jun 30, 2021

What you think about defining local types at the top of the file so it can be easily maintained, so you can merge and fix later on?

type Auth = firebase.default.auth.Auth;

@ShaMan123 ShaMan123 closed this by deleting the head repository Sep 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants