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

Firestore Collection to map rather than list #685

Open
AutomateAaron opened this issue Apr 8, 2020 · 8 comments
Open

Firestore Collection to map rather than list #685

AutomateAaron opened this issue Apr 8, 2020 · 8 comments
Labels

Comments

@AutomateAaron
Copy link

AutomateAaron commented Apr 8, 2020

I have the following binding from my Firestore collection to my state:

state: {
  assets: [],
},

actions: {
  bindAssets: firestoreAction(({ bindFirestoreRef }) => {
    return bindFirestoreRef('assets', db.collection('assets'))
  }),
},

However this binds them to assets as a list, I.e.

[
  {id: "id1" /* etc. */ },
  {id: "id2" /* etc. */ },
  {id: "id3" /* etc. */ },
]

Whereas I'd like it to be bound as a map like:

{
  "id1": { /* etc. */ },
  "id2": { /* etc. */ },
  "id3": { /* etc. */ },
}

How could I go about achieving this?

@posva posva added feature request firestore new Cloud Store labels May 18, 2020
@posva
Copy link
Member

posva commented May 18, 2020

You can create a computed that generates the object from the array. More information would be helpful to decide whether this feature is worth adding or not 🙂

@dannydulai
Copy link

I have a list of documents from a query, and I don't need the entire document in memory, just one of the ids

is there a way to use serialize option to solve this?

@rendomnet
Copy link

rendomnet commented Jul 9, 2020

@posva
I also need this feature. It is more handy to store large data in object. So we can easily refer to specific document from collection like this todos[id]status rather than search array.

@ajgagnon
Copy link

ajgagnon commented Sep 28, 2020

I'm sure this would involve a major, breaking change since all the mutations are already based on arrays. But it's definitely better architecture for performance reasons too.

Based on @posva, you could do a computed property like this:

computed: {
  assets: {
      get(array) {
        return array.reduce((obj, item) => {
          obj[item.id] = item;
          return obj;
        }, {});
      },
      set(value) {
        // covert back to array
        this.$store.commit('SET_ASSETS', Object.values(value));
      },
    },
}

It doesn't solve the performance issue since you're actually doing MORE computations, but will ensure:

  1. You have no documents with duplicate ids
  2. You can access assets like this.assets[id]

@azeem-r00t
Copy link

Has there been any update to this?

It is not just about a computed property. By not storing the collection as a map, I feel like we are not being true to how firestore stores these collections. In firestore, collection is not a list of documents rather it is a dictionary of ids mapped to specific documents. Because, this ends up being a generic list, it creates some interesting challenges when subcollections are being used. In my case, I have got a subcollection that is being loaded as each component mapped to a document from the parent collection loads. I need to store these subcollection somewhere mapped by the original collection id in the state. It has proven to be an unnecessary hassle thus far (and I have had do a bunch gymnastics to get it right). It would be great if this was changed.

@charles-allen
Copy link
Contributor

Based on @posva, you could do a computed property like this:

computed: {
  assets: {
      get(array) {
        return array.reduce((obj, item) => {
          obj[item.id] = item;
          return obj;
        }, {});
      },

FYI if you're using lodash:

getters {
  keyedAssets: (state) => keyBy(state.assets, 'id')
}

Though obviously building a bindMapRef() into the framework will be more efficient (since VueFire/VuexFire must already be enumerating the snapshot). +1 :)

@charles-allen
Copy link
Contributor

charles-allen commented Jan 4, 2021

I have a weaker case, but I'd like to also throw it out for discussion...

Consider I have another type that references assets, such as comment: { assetId: 123, ... } (i.e. many comments per asset). If I wanted to download comments related to multiple assets at once (e.g. a page showing multiple assets + their comments, or if I wanted to cache everything in Vuex), I'd probably want my comments stored like this:

{
  assetId1: [comments],
  assetId2: [comments],
  ...
}

Today I would do this in 2 steps:

  1. Bind all relevant comments: bind('comments', db.collection(COMMENTS).where(...))
  2. Use a getter to group them: get commentsByAsset() { return groupBy(this.comments, 'assetId') }

I think we could avoid the overhead of creating & storing the comments array if VueFire did the groupBy internally during snapshot parsing (similar to the OP's proposed change), eg: bindGroupBy('commentByAsset', ...). That said, an obvious argument against this is API-bloat!

@posva posva mentioned this issue Oct 13, 2022
43 tasks
@posva posva added the vuex label Nov 18, 2022 — with Volta.net
@posva posva removed the vuex label Jul 7, 2023
@posva posva changed the title Vuexfire Bind to Firestore Collection to map rather than list Firestore Collection to map rather than list Jul 7, 2023
@Mortenjoe
Copy link

Is this the only feature request that got away from the #1241 Roadmap or is it still under consideration? I can see the Roadmap-issue is closed.

When the data model does not imply an array, the above mentioned solution diminishes Vuefires sleekness and I find myself "manually" binding the collection without Vuefire. The withConverter() seems to wrap the results with an array in spite of returning an object.

Best regards

Morten

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

No branches or pull requests

8 participants