v1.3.0 is shaping up to be a somewhat sizeable set of changes:
createAsyncThunk and createEntityAdapterredux-immutable-state-invariant overredux-devtools-extension package to see if we can improve shakeability? (or maybe things can get improved upstream)Also, we should totally try to use https://github.com/mweststrate/import-size to help improve our shaking and bundle size.
Sketch of tasks to do:
redux-immutable-state-invariantredux-devtools-extensioncreateAsyncThunk and createEntityAdaptercreateAsyncThunk error handling finalized and documentedcreateAsyncThunk: data fetching in general, standard Redux patterns, how this simplifiescreateEntityAdapter: normalization in general, a couple recipes for how you could do normalizing yourself and then use these methods to process (like calling normalizr in an action creator, and having a slice check for action.payload.entities.users and process accordingly)rtk@^1.3.0Ongoing v1.3.0 integration work is in this PR:
Additionally:
@markerikson how about using keys instead of ids? We already using entity and entities, so I think we should follow common datastore Entities, Properties, and Keys naming. What do you think?
@chybisov : I'm not sure what you mean by "common naming".
The @ngrx/entity API already has state.entities and state.ids, which I like better than the docs page I wrote suggesting state.byId and state.allids:
https://redux.js.org/recipes/structuring-reducers/normalizing-state-shape
So no, I don't see any reason to change that.
@markerikson I mean standard way to name such things. In database world we have entities and keys to this entities. Pretty straightforward names. Key is more generic name, than id, it would be better.
btw @ngrx/entity already uses keys as variable name in a lot of place, I don't know why they not change the main one. I think most people just don't care :) but for consistency it would be just great.
I feel like id is more relevant, given that we're also assuming the values have entity.id to begin with. Not planning to change that.
I'm inclined to punt on the new Immer version for now. Whenever it comes out, it's probably worth its own separate update, probably 1.4.0. Removing that from the list.
Throwing this here for now because I don't want to open another issue atm. Someone on Reddit showed some interesting notional syntax for generating async action types inside of createSlice:
https://i.imgur.com/kcJUcxd.png
const someSlice = createSlice({
name: "slice",
initialState,
reducers: {
fetchItems: {
success(state, action) {}
}
}
})
Not sold on it, but it does go along with us already having an object notation for prepare callbacks. Worth noting.
Not sold on it, but it does go along with us already having an object notation for
preparecallbacks. Worth noting.
I'd +1 this suggestion - personally using sagas and no thunks and I ended up wrapping createAction so that the returned action creator returned plain "starting" action and had the creators equivalent to async stages as properties. Essentially ended up with creators like fetch(), fetch.pending(), fetch.resolved(), fetch.rejected(), fetch.cancel().
I quite like how it feels in development, but my API is a tad bit awkward, not to mention I had to do the same thing with reducers (yeah, wrapping createSlice isn't easiest).
I'm pretty confident it could be done though, and having this as a core feature of RTK would be awesome for those of us who use sagas, observables or similar, instead of thunks :)
I love the entityAdapter! 鉂わ笍
I think that a couple more selectors could be added to it though. Namely selectOne and selectMany. It'd better map with the CRUD operations (where you have addOne, addMany, removeOne, etc...). Moreover, I think it would be useful in overall applications where you have a page listing all the entities and a details page displaying one entity.
What do you think?
@CosmaTrix @markerikson it would be very useful, especially selecting some object by id or some other property.
Yeah, I'd definitely been thinking about adding a selectById. Pretty simple to add. Not sure what selectMany would look like, API-wise. Would it take a list of IDs as a selector param?
@markerikson Yup, that's my idea.
// types
selectOne: (state: V, id: EntityId) => T | undefined
selectMany: (state: V, ids: EntityId[]) => Dictionary<T>
// examples
selectors.selectOne(state, 1);
selecttors.selectMany(state, [1, 2, 5]);
I've cloned the repo today and played a little bit with the source code. I've added, typed and tested selectOne and selectMany. The only thing left to do, I guess, would be to update the documentation and then create a PR. Shall I go for it?
I wouldn't expect selectMany to return a lookup table, but rather an array.
My main question for this is the typical memoization issue. Notionally, an implementation of selectMany would look like:
const selectMany = createSelector(
selectEntities,
(state, ids) => ids,
(entities, ids) => ids.map(id => entities[id])
)
The problem is that a typical usage pattern like selectMany(state, [1, 2, 5]) would pass in a new ids array every time, which would cause the output selector to _always_ recalculate, which would _always_ return a new array reference as a result. Same thing if it's getting used in multiple places to with varying inputs.
(This has always been the weakest aspect of Reselect. Since it only caches the latest calculated value, changing the inputs causes recalculations, which can both be expensive in terms of transformation cost, and returning new references as a result typically causes re-renders at the UI level.)
The current list of generated selectors doesn't have to worry about that, because the only inputs are the EntityState object. Even selectById is okay, because it's a straight lookup. Calling it multiple times with different IDs will technically cache-bust, but it's okay because there's no transformation being done.
Not saying we can't add it, but it's a usage nuance that gets tricky.
Tell you what. I'll go ahead and add selectById to the next beta, and we can talk about whether it's worth adding some kind of selectMany from there.
@markerikson You're so right! Both about the return type of my selectMany (it should in fact be selectMany: (state: V, ids: EntityId[]) => T[]) and about its memoization "issue" with an array being always a different input.
But glad to hear you're going to add the selectById selector. 馃憤
Yeah, just put that out in https://github.com/reduxjs/redux-toolkit/releases/tag/v1.3.0-beta.1 .
v1.3.0 is now LIVE! https://github.com/reduxjs/redux-toolkit/releases/tag/v1.3.0
We don't have the usage guide docs for createEntityAdapter in yet, but I'm going to close this and add separate issues for the docs work.
Most helpful comment
Tell you what. I'll go ahead and add
selectByIdto the next beta, and we can talk about whether it's worth adding some kind ofselectManyfrom there.