feat: dynamic clientId Initialization #61
Conversation
|
Thanks @zuhno this is a brilliant work, and im really happy about this PR 💪 it looks really good as i checked on surface |
kasvith
changed the title
|
@kasvith Please let me know if there are any parts that need to be revised. |
|
Sure, i didnt have much time lately to focus on the project |
kasvith
changed the title
src/composables/useTokenClient.ts
Outdated
| const isReady = ref(false); | ||
| let client: TokenClient | undefined; | ||
|
|
||
| const login = (overrideConfig?: OverridableTokenClientConfig) => { | ||
| if (!isReady.value) { | ||
| if (!clientId?.value) { |
There was a problem hiding this comment.
seems we are repeating this in multiple places, can we perhaps extract it to a composable? like useClientId?
There was a problem hiding this comment.
would useClientId be a hook intended solely for internal use within the library? If it’s not meant to be exposed externally, I think it might also make sense to modularize the validation part within the login function as a utility function.
There was a problem hiding this comment.
i think for now it can be used internally
yes, refactoring validation part would be nice too
There was a problem hiding this comment.
completed the refactoring.
please review the code, and let me know if any further adjustments are needed 😀
src/utils/validations.ts
Outdated
| @@ -0,0 +1,22 @@ | |||
| import { toPluginError } from "./logs"; | |||
|
|
|||
| export const validateLoginSetup = (isReady: boolean, clientId?: string) => { | |||
There was a problem hiding this comment.
shall we call it isClientIdValid ?
also lets get rid of the nested if statements and use early returns
There was a problem hiding this comment.
as you mentioned, we can use an early return. in doing so, the throw new Error would become non-executing code, so if there's no need to throw an error, using early return could be a good approach.
There was a problem hiding this comment.
we can throw error as early return
There was a problem hiding this comment.
applied early return, which allowed me to improve the nested if statements
refactor: applied early return to 'isClientIdValid'
Hi :)
setGoogleClientId, which operates by changing the global ref value.GoogleSignInButtonis called.I am very happy to contribute to this project. Please let me know if there are any suggestions for improvements in the folder structure or functionality.