prescottprue / redux-firestore

Redux bindings for Firestore
MIT License
575 stars 102 forks source link

feat(reducers): switch ordered to be just be keys instead of objects #204

Open jaschaio opened 5 years ago

jaschaio commented 5 years ago

I am not a 100% on this one, but I can't immediately see why the same data is repeated between the firestore.data and firestore.ordered. This essentially doubles the size of the store and I can see how this becomes problematic for large apps with large documents if I want to persist state in localstorage.

Wouldn't it be enough for firestore.ordered to be simply an array of IDs so that I know the order of IDs and then get the data from the firestore.data key?

prescottprue commented 5 years ago

Good question! This is intentional by design so you can look items up by key in data where as ordered is for an ordered list (from calling forEach on the query response from firebase). It should in theory be enough for firestore.ordered to be just keys, but since the full data is available when dispatching the action, it was included in state.

Since this would be a massively breaking change for some applications we would need to put it into a new major version. Definitely interested though, especially if we can show some measure of less memory usage.

As for the "large store" problem, I have worked on a few applications with MASSIVE redux state trees, but did not really run into issues unless there there was expensive logic being run for all state updates or for large pieces of state (also backed up by the performance section of the redux docs).

When persisting state into local storage, it is best to only include what parts of state you need anyway since stringifying/parsing can be expensive. From what I understand, it is more common to use something like indexedDb (which Firebase uses for some of its internals), or webSQL as the persistence engine.

jaschaio commented 5 years ago

I actually wondered about this because I designed my store previously for using another REST API and used the allIds and byIds approach described within the redux docs.

allIds is mostly equal to the ordered key without contents and byIds to the data key just as it is. If normalizing state this way is indeed the best practice it probably makes sense to change this for the next mayor version.

Getting back the same ordered data is still trivial as one just joins together both state keys:

const ordered = ( allIds, byId ) => allIds.map( ( id ) => byId[ id ] );

I basically got the same use case as described in the docs: large objects with lots of content (think of blog posts). Even having just a few pages worth of data in the store can get pretty huge memory wise.

In my case it gets even worse as I am repeating the data a third time joining together various pages via saveAs: page.2 and so forth within a single byId state key. It might make sense to take pagination in mind, the saveAs + page actually feels hacky but I couldn't find another simple approach within redux-firestore right now for handling pagination and that was the one turning up in gitter.

prescottprue commented 5 years ago

Yeah like I said, I agree that it is best practice to keep just the keys in the array, and that is what I use with other APIs anyways, so lets switch this to a ticket for capturing that. As always, I'm open to PRs if you get a chance to work on it.

Really glad to have folks giving input on the core architecture, especially on redux-firestore since it is still so young. Thanks for reaching out!

illuminist commented 4 years ago

I'd like this too, but how are we going to handle nested collections? I personally always use storeAs so each query even the deeper one has its own state at root of data and ordered state.

prescottprue commented 4 years ago

@illuminist In v1 the all subcollections are stored at the top level with the path in state including the name of the subcollection. With that in mind, all of the ordered state should just be a single level with different arrays of keys regardless of what level the in the collection/subcollection tree then data lives