Skip to content
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

useFirestore for docs, collections or queries that include an auth users id in the path #2018

Closed
4 tasks done
smakinson opened this issue Jul 31, 2022 · 6 comments · Fixed by #2103
Closed
4 tasks done
Labels
enhancement New feature or request

Comments

@smakinson
Copy link

smakinson commented Jul 31, 2022

Clear and concise description of the problem

Currently useFirestore expects to be given a doc, collection, query or reactive that supplies one of these. It would be nice if it were possible to use useFirestore for any of these that are built using an authenticated user id as part of the path without concern for if the user has let authenticated or not. Since this user id is not known until authenticated, currently additional checking must be used to wait until the user has authenticated to call doc(db, 'users', 'my-user-id') or collection(db, 'users', 'my-user-id', 'another-collection'). It would be nice to be able to call useFirestore at any time and the ref it returns would contain the initial value until the user has authenticated.

Suggested solution

It would be nice if useFirestore or perhaps a new composable useAuthFirestore could be called at anytime and a ref would be provided that has its value set to an initial value until the user has authenticated. This would avoid code in various places waiting on authentication. If the user signed out, the ref value would return to the initial value...likely []. This composable would also take either the params similar to useAuth or perhaps the return value of an already called useAuth. It would be able to tell when authenticated and get the user id from this.

Perhaps instead of expecting a doc, collection, query or reactive providing one of these, an array could be provided with the db & portions of the paths. A placeholder for where the user id would go could be provided in the array, perhaps something like: [db, 'users', null, 'some-collection'] It would be known whether to use collection or doc based on the number of items in the array. Perhaps to handle the query scenario a reactive function could be provided that would return an array with everything to be used as a parameter to query after the first collection or doc param....since the composable already knows how to create that portion, for example it might return an array with something like is shown in the docs for a reactive query: [orderBy('createdAt', 'desc'), limit(postLimit.value)]

Alternative

Writing additional conditional code that waits until the user has authenticated before calling useFirestore. Another alternative that allows a ref to be used before useFirestore has been called is something like:

const myRef = ref([])

// wait until the user has authenticated and do something like:
stopSync = syncRef(myRef, useFirestore(collection(db, 'users', 'my-user-id'), []))

Additional context

No response

Validations

@smakinson smakinson added the enhancement New feature or request label Jul 31, 2022
@smakinson
Copy link
Author

smakinson commented Aug 7, 2022

Something came to mind, so I'll post it here. The above doesn't account for converter usage. Perhaps [db, 'users', null, 'some-collection'] Should be more along the lines of: [db, doc, 'users', null, 'some-collection'] or [db, collection, 'users', null, 'some-collection'] or [db, myCollectionWithConverter, 'users', null, 'some-collection'], removing the need for useFirestore to determine what to create. This is a bit of thinking out loud, definitely open to ideas.

@kiyopikko
Copy link
Contributor

In my opinion,

useAuthFirestore

firebase/firestore and firebase/auth are modules. They are independent and work separately. It's better to avoid to use multiple modules.

[db, 'users', null, 'some-collection'] or [db, doc, 'users', null, 'some-collection'] or [db collection, 'users', null, 'some-collection'] or [db, myCollectionWithConverter, 'users', null, 'some-collection'] or [db collection, 'users', orderBy('createdAt', 'desc'), limit(postLimit.value)]

seems too complicated interface. There are too many patterns and hard to handle them. It's better to keep it simple, just the same interface as onSnapshot.

@smakinson
Copy link
Author

smakinson commented Aug 9, 2022

Thanks for the reply. I agree it feels complicated looking at the array arg possibility. Initial thought was to have something that was identifiable for the if/else checking in on the 1st arg in useFirestore. Perhaps something like a function given as the first arg that would be called when the user signed in and it would be responsible for handing back to useFirestore one of the same types that can be passed as the 1st argument as it is now. One other addition for useFirestore would be giving it the isAuthenticated ref that the calling code received from useAuth. Still just tossing out ideas, but maybe it would look something like:

const { isAuthenticated, user } = useAuth()

const myRef = useFireStore(() => {
  // this could return anything that useFirestore currently accepts for the first argument.
  return collection(db, 'users', user.value.id, 'some-collection')
}, [], { isAuthenticated })

Initially myRef.value is [], once the isAuthenticated ref has a value of true, the 1st argument is called and that is used to set the value of myRef as it happens now in useFirestore. When the value of isAuthenticated returns to false, the initial value is then set back into the value of myRef.

@kiyopikko
Copy link
Contributor

I'm not native of English so not completely clear of your reply. but the below interface is possible and maybe what you want.

const todos = useFirestore(collection(db, 'todos')) // works as usual
const todo = useFirestore(doc(db, 'todos', 'todo-id')) // works as usual
const postsQuery = computed(() => query( /* ... */ ))
const posts = useFirestore(postsQuery) // works as usual

const { user, isAuthenticated } = useAuth()

// new interface
const todos = useFirestore(() => collection(db, 'todos')) // same as passing directly collection reference
const userData = useFirestore(() => isAuthenticated.value && doc(db, 'users', user.value.id), null)
// isAuthenticated is false: return null (initial value)
// isAuthenticated is true: bind snapshot
  1. care about compatibility
  2. same as useSWRV Dependent Fetching impl

@smakinson
Copy link
Author

smakinson commented Aug 10, 2022

Yes, thank you, that new interface looks correct. The one additional request is that in the example below, userTodosData.value would automatically be set to the initial value again when the user signed out. This the reason the example above added isAuthenticated to the UseFirestoreOptions parameter.

const { user, isAuthenticated } = useAuth()

const userTodosData = useFirestore(() => isAuthenticated.value && collection(db, 'users', user.value.id, 'todos'), [])

// or this would allow useFirestore to automatically reset the ref value to the intial value when the user signs out:

const userTodosData = useFirestore(() => isAuthenticated.value && collection(db, 'users', user.value.id, 'todos'), [], { isAuthenticated })

The flow would be:

  • User comes to website, but has not yet signed in. At this point userTodosData.value is []
  • User signs in. userTodosData.value is now the data from Firestore.
  • User signs out. userTodosData.value is now the initial value again []

Please let me know if this makes sense and thank you again.

@kiyopikko
Copy link
Contributor

Yes,

const userTodosData = useFirestore(() => isAuthenticated.value && collection(db, 'users', user.value.id, 'todos'), [])

probably this also can set the initial value again when 1st arg gets falsy value without isAuthenticated option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants