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

Feature: shortcut to get subcollection for a document #89

Open
gsouf opened this issue Feb 18, 2020 · 11 comments
Open

Feature: shortcut to get subcollection for a document #89

gsouf opened this issue Feb 18, 2020 · 11 comments

Comments

@gsouf
Copy link

gsouf commented Feb 18, 2020

The official firestore client gives us ability to get reference to a subcollection in this way:

var messageRef = db.collection('rooms').doc('roomA')
                .collection('messages').doc('message1');

I couldn't find a equivalent way to do that with firestorter. Is it by design or is it something we can add? In that case I'm happy to send a PR

@IjzerenHein
Copy link
Owner

Hi. You can use use paths like this:

new Collection('rooms/roomA/messages');

And for document as well:

new Document('rooms/roomA/messages/message1');

@gsouf
Copy link
Author

gsouf commented Feb 19, 2020

Hi @IjzerenHein

yes, that's what I did in the interim, but when you have an existing reference to a document it saves the burden of doing a string concatenation.

new Collection(`${myDocRef.path}/something`);

// vs:

myDocRef.collection('something');

@IjzerenHein
Copy link
Owner

Right okay, I don't really see much benefit of that myself tbh.
But okay, what kind of API update do you have in mind? Please be specific and provide an API proposal.

@gsouf
Copy link
Author

gsouf commented Feb 19, 2020

@IjzerenHein thinking of adding a method collection that would return a ref to the collection with the path from the parent + collection name, and a method doc returning a ref to the sub document with the parent. Actually the same way as the official firestore client.

@IjzerenHein
Copy link
Owner

Can you provide some sample code that shows the API usage and how you would use it?

@gsouf
Copy link
Author

gsouf commented Feb 22, 2020

Sure...

Given we have a doc ref

const docRef = new Document('myCollection/abcd');

This is how we do now:

const subCollectionRef = new Collection(`${docRef.path}/subCollection`);

This is the proposal:

const subCollectionRef =docRef.collection('subCollection`);

Additionnally we could do the following:

// get a document for the subcollection
const subCollectionDocRef = docRef.collection('subCollection').doc('some id');

// ability to nest even deeper
const subSubCollectionDocRef = docRef.collection('subCollection').doc('some id')
   .collection('subSubCollection').doc('som id');

That's equivalent to this sample from official client https://github.com/firebase/snippets-web/blob/c34434c12d6db375f0fb1ed71031126851f199b7/firestore/test.firestore.js#L177-L178

@IjzerenHein
Copy link
Owner

IjzerenHein commented Feb 22, 2020

I've been so free to re-format the code and remove the word "ref" from it. I find "ref" confusing and unnecessary here and implies that it is the same thing as a Firestore Collection/Document reference, which it is not.

Sure...

Given we have a doc

const doc = new Document('myCollection/abcd');

This is how we do now:

const subCollection = new Collection(`${doc.path}/subCollection`);

This is the proposal:

const subCollection = doc.collection('subCollection`);

Additionally we could do the following:

// get a document for the subcollection
const subCollectionDoc = doc.collection('subCollection').doc('some id');

// ability to nest even deeper
const subSubCollectionDoc = doc.collection('subCollection').doc('some id')
   .collection('subSubCollection').doc('som id');

That's equivalent to this sample from official client https://github.com/firebase/snippets-web/blob/c34434c12d6db375f0fb1ed71031126851f199b7/firestore/test.firestore.js#L177-L178

@IjzerenHein
Copy link
Owner

IjzerenHein commented Feb 22, 2020

This is how we do now:

const subCollection = new Collection(`${doc.path}/subCollection`);

There is actually a slightly better way of doing this, that also deals with dynamically changing paths, which will be relevant in the next comment. You can also do this:

const subCollection = new Collection(() => `${doc.path}/subCollection`);

This will cause the collection to automatically update its path whenever the path in the document changes. This can be very useful, especially when you have a chain of collections/document that depend on each other. Changing the one at the top can cause child collections/documents to automatically update.

@IjzerenHein
Copy link
Owner

IjzerenHein commented Feb 22, 2020

Generally, I like your idea. I think the syntax is neat and compact. I do see some issues/questions though that we need to think about:

  1. As I mentioned before, you can use "reactive" paths as the source for a collection or document. When doing doc.collection('bla'), we could create it in two ways:
class Document {
  collection(path: string) {
    return new Collection(`${this.path}/${path}`);
  }
}

or like this:

class Document {
  collection(path: string) {
    return new Collection(() => `${this.path}/${path}`);
  }
}

Or possibly allow both through an extra argument. Anyway, we would have to choose a default one.

  1. Secondly. We'd need to support Generics for this as well. So imagine that your document or collection, uses a certain type:
interface MessageData {
  userId: string,
  test: string
};

const messages = chat.collection<DocData>('messages');

When you omit the generic-type, we could use any.

What are your thoughts?

@gsouf
Copy link
Author

gsouf commented Feb 23, 2020

Thanks for the explanations.

Actually I'm not sure what default would fit better between the simple string approach vs the reactive path. I just started to work on an application using firestorter so I still lack of experience with it to give the right answer. If you recommend usage of reactive path then we probably want to use this one. Because the library is focused on doing things automatically then it's probably the right way to go to keep better consistency.

For generics, that makes sens.

@IjzerenHein
Copy link
Owner

Actually I'm not sure what default would fit better between the simple string approach vs the reactive path. I just started to work on an application using firestorter so I still lack of experience with it to give the right answer. If you recommend usage of reactive path then we probably want to use this one. Because the library is focused on doing things automatically then it's probably the right way to go to keep better consistency.

Okay, in that case I will put this feature request on hold for now. I don't want to expand the API footprint with optimisation features that might not get used.

@IjzerenHein IjzerenHein changed the title shortcut to get subcollection for a document Feature: shortcut to get subcollection for a document Feb 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants