-
Notifications
You must be signed in to change notification settings - Fork 307
Fix auth bug #98
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
Fix auth bug #98
Changes from all commits
8a525c5
73aeb30
6638911
c1a0d20
5c79f11
8c5e15d
b23480d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,3 +13,6 @@ npm-debug.log | |
# Testing output | ||
.nyc_output | ||
coverage | ||
|
||
#test files | ||
/example |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,3 @@ | ||
export { default as useAuthState, AuthStateHook } from './useAuthState'; | ||
export { default as useLogin, loginHook } from './useLogin'; | ||
export { default as useRegister, registerHook } from './useRegister'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
import { useState, useMemo } from 'react'; | ||
import firebase from 'firebase/app'; | ||
import 'firebase/auth'; | ||
import { AuthHookType } from '../util'; | ||
|
||
export type loginHook = AuthHookType< | ||
firebase.auth.UserCredential, | ||
firebase.FirebaseError | ||
>; | ||
|
||
export default ( | ||
auth: firebase.auth.Auth, | ||
email: string, | ||
password: string | ||
): loginHook => { | ||
const [error, setError] = useState<firebase.FirebaseError>(); | ||
const [ | ||
loggedInUser, | ||
setLoggedInUser, | ||
] = useState<firebase.auth.UserCredential>(); | ||
const [loading, setLoading] = useState<boolean>(false); | ||
|
||
const login = () => { | ||
setLoading(true); | ||
auth | ||
.signInWithEmailAndPassword(email, password) | ||
.then((resUser) => { | ||
setLoggedInUser(resUser); | ||
setLoading(false); | ||
}) | ||
.catch((err: firebase.FirebaseError) => { | ||
setError(err); | ||
setLoading(false); | ||
}); | ||
}; | ||
|
||
const resArray: loginHook = [loggedInUser, error, login, loading]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From a return perspective, I think that the function should be returned as the first item in the array and the order of the other elements should match the order of So it should be: |
||
return useMemo<loginHook>(() => resArray, resArray); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
import { useState, useMemo } from 'react'; | ||
import firebase from 'firebase/app'; | ||
import { AuthHookType } from '../util'; | ||
|
||
export type registerHook = AuthHookType< | ||
firebase.auth.UserCredential, | ||
firebase.FirebaseError | ||
>; | ||
|
||
export default ( | ||
auth: firebase.auth.Auth, | ||
email: string, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As above, I think it would be better to make |
||
password: string | ||
): registerHook => { | ||
const [error, setError] = useState<firebase.FirebaseError>(); | ||
const [ | ||
registeredUser, | ||
setRegisteredUser, | ||
] = useState<firebase.auth.UserCredential>(); | ||
const [loading, setLoading] = useState<boolean>(false); | ||
|
||
const register = () => { | ||
setLoading(true); | ||
auth | ||
.createUserWithEmailAndPassword(email, password) | ||
.then((resUser) => { | ||
setRegisteredUser(resUser); | ||
setLoading(false); | ||
}) | ||
.catch((err: firebase.FirebaseError) => { | ||
setError(err); | ||
setLoading(false); | ||
}); | ||
}; | ||
|
||
const resArray: registerHook = [registeredUser, error, register, loading]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As above, the ordering should match what's proposed for |
||
return useMemo<registerHook>(() => resArray, resArray); | ||
}; |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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 it would be better to make
email
andpassword
variables that are passed into the returnedlogin
function. This is more in keeping with what some of the other libraries that allow mutation of data, e.g. apolloThere 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.
Actually I was thinking about the exact same thing