-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
More advanced access patterns for createEntityAdapter
?
#799
Comments
I can definitely see the potential use cases for maintaining additional indices. That said, this is the first time anyone's asked for something like that. So, my off-the-cuff response is that I'd like to see some API proposals and proof of concepts first, and then discuss specifics and whether it's something that's actually worth including in RTK directly. Frankly, we've already gone well past a couple limits that I had originally listed for RTK :) I wrote up a "My Vision for RTK" statement almost two years ago, and at that time I said:
Well... we've already got |
Thank you for the great response. As for an API proposal, here's a first draft of what I'd imagine, with a few possible options for each part: // initialisation
const entitiesAdapter = createEntityAdapter<TEntity>({
selectId: (entity) => entity.id,
sortComparer: (a, b) => b.createdAt.localeCompare(a.createdAt),
indexes: {
whereParentIsNull: {
byFilter: (entity) => entity.parent === null,
},
byParent: {
// as function, easy to type
byField: (entity) => entity.parent,
// easier to use, not sure if possible to type via inference
// both are probably necessary as being able to derive the field may be useful?
byField: 'parent',
}
},
// or
indexes: [
{
name: 'whereParentIsNull',
byFilter: (entity) => entity.parent === null,
},
{
name: 'byParent',
byField: (entity) => entity.parent,
// or
byField: 'parent',
}
]
});
// regular actions like addOne handle keeping the indexes up to date
// maybe a `skipIndex` param may be useful but I wouldn't know why
// Access
// Export the customised selectors for this adapter using `getSelectors`
export const {
selectAll: selectAllEntities,
selectById: selectEntityById,
selectIds: selectEntityIds,
// Pass in a selector that returns the entities slice of state
} = entitiesAdapter.getSelectors<RootState>((state) => state.entities);
// What I currently have
export const selectEntityIdsByParent = (parent: NonNullable<TEntity['parent']>) =>
createSelector(
(state: RootState) => state.entities,
(state) => state.idsByParent[parent] || [],
);
export const selectEntityIdsWithNoParent = createSelector(
(state: RootState) => state.entities,
(state) => state.idsWhereParentIsNulll,
);
// What we could have
export const selectEntityIdsWithNoParent = entitiesAdapter.getIndexSelector<RootState>('whereParentIsNull')
export const selectEntityIdsByParent = entitiesAdapter.getIndexSelector<RootState>('byParent')
// Or we could export the selectors as the names of the indexes
// So index names are `selectWhereParentIsNull` and `selectByParent`, and then we have:
export const {
selectAll: selectAllEntities,
selectById: selectEntityById,
selectIds: selectEntityIds,
selectWhereParentIsNull: selectEntityIdsWhereParentIsNull,
selectByParent: selectEntityIdsByParent
// Pass in a selector that returns the entities slice of state
} = entitiesAdapter.getSelectors<RootState>((state) => state.entities); I'm not sure how easy this would be to type, I've not delved in to any of the more advanced generic and inference stuff in TS yet. As for a proof of concept, what form would that take? A rough implementation within this codebase? Tests with no implementation TDD style? |
Mm... first question, I think, is about the actual state design and behavior. What form should these additional indices take? Objects? Arrays? Dealing with just the IDs, or the actual item values? Are they supposed to be sorted in some way? Are these supposed to be kept updated after every operation? Beyond that, it's always easiest to talk about actual semi-working code :) agreed that the TS typing is probably going to be the most difficult part atm, but if you can try tackling it and fake out some of the types for now just to get the code actually updating whatever state you're envisioning, that would give us a starting point for further discussion. |
Both. Could be an array of IDs filtered, or maybe even sorted too, a different way, or an object with keys derived from the data equalling arrays of IDs filtered and sorted.
In state, just IDs. Along with the selectors that return arrays of IDs, we could also have ones that return the full entities if a user so desires.
I think the option should be there, the same way it is for the regular index of entities the adapter creates.
Yes, they would be kept updated automatically after every operation by the adapter itself. Since it's all just based on the data the user is already putting in through the regular entity adapter, we could, and I started with this, create selectors that use map, filter, and sort to get this same information, in the same way we could get the entityIds field from the normal adapter to get an array of all the IDs, but that just doesn't quite feel right from a performance perspective.
Fair enough. I'll see if I can look at getting to grips with the codebase and getting some sort of implementation going, although I don't know when that'll be as it'll probably have to be outside of work hours for now. I'll update this issue as and when. |
I like the mentioned approach but had a different one in mind to abstract the filtering a bit. if you allow me. {
"ids": [],
"withParent2": [],
"withParentNull": [],
"entities": {}
} If we define an extra parameter for the CRUD operations functions as follows: updateMany(state: S, updates: PayloadAction<Update<T>[]>, key: string) Key values could be If you like this approach I can start working on a codebase right now and submit a pull request. |
Adding extra args here won't work because those functions need to work as standalone reducers. |
@markerikson forgot about this part 😅 I'll try and come up with a solution and update when done. Thank you for the amazing package! |
FWIW, I'm still open to this and any other ideas for improving |
For my use case, on search I get the new list from server. I think simple approach to tackle most use cases would be createAdapter
initial state
reducer
search result reselect selectors can be created from entities selector and state.searchResultIds. |
Sorry, I'm not sure what API or behavior changes you're trying to show in that example. Can you clarify what you're describing? |
As you can see in my example I am trying to maintain 2 sets of ids searchResultIds and defaultIds but both share the same entity map. |
Okay, but I don't understand what you're proposing in terms of a second "ID index" here. It would help if you could describe what you're expecting to have happen, preferably with some examples for what those return values would contain. |
What I am trying to say is if all the update methods of createEntityAdapter (updateOne, updateMany, ...) can return the id list and the entity object (as an object or array) then we can derive the desired id list from them and maintain it in a separate key in the store. Selectors for them can also be created easily by composing custom selector with entityAdapter selector. |
Not really sure how that helps. For one thing, the API adapter CRUD methods already have defined return values, so we can't change those. Second, the CRUD methods either "mutate" the existing Third, the updated state either way is going to have the complete list of IDs, not just some subset. So, I'm still not clear what specific results you're expecting to see here that would make additional indices easier to manage, and we can't change the API signatures because that would be a breaking change. If you could give me a specific example (fake code, but actually showing example values in the arrays and results and stuff), it might help me understand what you're asking for here. |
For @lukeramsden's problem I think it's not a good idea to maintained filtered id list in state. Rather that can be done using an reselect selector.
|
There are times when maintaining an additional filtered list of IDs might be useful. See https://medium.com/@dcousineau/advanced-redux-entity-normalization-f5f1fe2aefc5 . |
I use immer in my project so missed the case where the update method returns the new state after update. Adding one method to get the normalised data will help. eg.
updated reducer
|
We're not looking to implement a complete normalization solution of arbitrary incoming data. For that, there's https://github.com/paularmstrong/normalizr , and we have a usage guide section that suggests how to use it with RTK: https://redux-toolkit.js.org/usage/usage-guide#normalizing-with-normalizr None of this seems to actually relate to keeping some additional sub-index via the entity adapter that I can see. Please re-read the original comment in this thread to see what the problem statement is that we're discussing. |
what if we allow a partial curried function
without the .key updateMany can continue to work as it is now |
Hmm. Okay, that's at least something concrete that I can look at, but I'm not following the intended behavior exactly. In particular, this "key" parameter that you're passing in seems like a very different concept than what's being discussed in the original comment.
The question at hand is if it's possible to define additional indices, such as (fake code): const booksAdapter = createEntityAdapter<Book>({
// Assume IDs are stored in a field other than `book.id`
selectId: (book) => book.bookId,
// Keep the "all IDs" array sorted based on book titles
sortComparer: (a, b) => a.title.localeCompare(b.title),
// FAKE EXAMPLE HERE
indices: {
byAuthor: (a, b) => a.author.localeCompare(b.author),
byYear: (a, b) => a.year - b.year
}
}) Those indices would then also be automatically updated as items are added, updated, and removed. What you're showing is something very different. It looks like you're trying to associate certain IDs with specific keys, but only doing so in specific operations? I'm really not sure what the semantics are that you're proposing here. I think the biggest difference is that the original discussion is looking at trying to auto-maintain these indices based on attributes that are already in the entity objects themselves, whereas your suggestion seems to be about attaching additional metadata instead (or something like that). As a side note: @lukeramsden , I missed that you'd liked to |
We can keep the structure exactly like https://medium.com/@dcousineau/advanced-redux-entity-normalization-f5f1fe2aefc5 . and not change the createEntityAdapter constructor at all. current structure
new structure
so now on say setAll
updated state
|
I still don't understand what this "key" thing is you're proposing. I don't know what the intended semantics are, and I don't know what purpose it's supposed to serve. I can't even consider adding something if I don't know what it's supposed to do and how it's supposed to work. At a minimum, you'd need to explain exactly what that does, what problem it solves, and how it's supposed to behave. |
I think memoized selectors are the best way to solve the original problem of accessing by different keys. Which is basically multiple views filtered from state data. Regardless let me try to solve the original problem by maintaining create entity const entitiesAdapter = createEntityAdapter<TEntity>({
selectId: (entity) => entity.id,
sortComparer: (a, b) => b.createdAt.localeCompare(a.createdAt),
}); AddEntities entitiesAdapter.key('idsWhereParentIsNulll').setAll(store, payload.filter((entity) => entity.parent === null)) Store structure
Basically, I don't think we should store the configuration of how to populate the derived id lists. Rather we should allow specifying the key which will keep the ID list. The reason being the filtering logic of the sub id lists may be dependent on some other key in the state or some prop which can't be passed with the configuration. This way entityAdapter will not have to take the responsibility of updating the sub id lists on every update. The problem which I am trying to solve is to maintain the list of ids in multiple keys for multiple views. Let's say for server side searching of an entity list. In the original key ids we can maintain the default list of ids and then on search we can extract the ids from search api response and keep it in searchResultIds key. This way we can maintain bot the state in store and once user clears the search query we can quickly render the default list from ids key without having to make another api call. |
I don't like this idea of doing |
I was just trying to think of a way to implement this using createEntityAdapter. I will get back if I can think of a better solution. |
My inclination at this point would be to see if we can leverage that https://github.com/corps/redux-indexers package linked in the original comment. If we did, we'd probably port the logic over to RTK instead of adding it as a dependency. And sure :) |
@lukeramsden : so I've got something sorta-kinda working over in #948 . Would appreciate it if you could try out installing the build from CodeSandbox CI in that PR, and try this out. Types still aren't right yet - it's showing fields in |
createEntityAdapter({
sortComparer: (a, b) => { } // omit,
indices: {
byId: (a, b) => http://a.id.localeCompare(http://b.id),
byTitleLength: (a, b) => a.title.length - b.title.length
}
})
{
ids: [],
entities: {},
indices: {
byId: [],
byTitleLength: []
}
}
I am not sure this is an "index" quite yet. A database index would provide an alternate lookup key and not just an efficient alternate scan method (sort). For example, // API (methods, not the data structures backing them)
const shortTitleIds = books.byTitleLength(5, 10); // this queries the index
// ['b_123', 'b_456', 'b_001']
books.entities['b_123']
// { title: 'FooBar', id: 'b_123' }
const allBookIdsSortedByTitleLength = books.byTitleLength(); // this scans everything and uses the index sort
// ['b_123', 'b_456', 'b_001', 'b_098', 'b_1234']
books.entities['b_1234']
// { title: 'really long title', id: 'b_1234' }
// Selectors can use the index to get IDs instead of just the primary ID key or list of IDs
// However, without a uniqueness constraint on the index can only return arrays of items
const booksWithShortTitles = selectEntitiesById(books.byTitleLength(6));
// [ { title: 'FooBar', id: 'b_123' } ]
// Passing index result to ID selector is a bit indirect so maybe a default selector-like method could be added for each index
const booksWithShortTitles = books.selectByTitleLength(6);
// [ { title: 'FooBar', id: 'b_123' } ] EDIT: I think a pretty good data structure for this would be a sorted array (balanced tree) that takes the comparison function and stores an object reference or a structure like Rough example implementing a getRange() value on top of this SortedArray implementation: https://replit.com/@alexkrolick/HonoredEasySorting#index.js |
Thank you for your work on redux-toolkit, it really is an amazing library.
I'm using
createEntityAdapter
to keep my data normalised and it's working very nicely, however, my use-case means not only do I need to access by ID, but also other fields. I'm currently having to maintain my own "indices" based on various fields of my entities.My entity looks like this:
I need to be able to quickly access a list of entities in one of 2 ways:
parent
isnull
, for the index of top level entities with no parentparent
is equal tox
, for the children of a certain entityI'm currently maintaining my own two indices for this.
I then have to make sure these are both kept in sync wherever I'm updating the adapter state - not to mention keeping them sorted, which I haven't got round to yet.
Add in keeping the list sorted by
createdAt
, and many more call-sites updating these entities due to pagination, refreshing, etc. its quite easy to introduce de-sync bugs. This could be handled as computed state with a selector, which is what I started with, however due to it being, in my case, a mobile app, sessions are long-lived and a user may fetch a lot of content, even if they're only accessing a little, due to the tree-like nature of my data.I'll probably introduce helper functions that take in the state, and maybe the adapter, and perform these actions as sort of wrappers for the CRUD functions - however, is it plausible that redux-toolkit could handle these sorts of use-cases natively? A "filtered ID array" index and a "ID array grouped by field" would be enough for my use-case, however there may be other use-cases it could open the door for. Some interesting inspiration for this is https://github.com/corps/redux-indexers with it's multi-level indexes, which is a nice little utility, and may work very well for this although I haven't tried it yet.
And I will also say in advance that I'm not adamant this is the best solution to my own use-case, or that this should definitely be handled by redux-toolkit, but I am interested on what people who have a lot more experience with React and Redux than me would have to say about this.
EDIT: another use-case could be having multiple indices for different sort options
The text was updated successfully, but these errors were encountered: