Open yangm97 opened 3 years ago
First of all both robodux and saga-query seem to very much have the correct opinions about state and side-effect management within react applications. Nice work!
Thank you!
Is a byColumnIds "index" reducer or selector considered to be within the scope of robodux? If so I can see if I can send a PR
Could you expand on what you mean by a byColumnIds
reducer or selector? If the goal of such a selector is to be able to find data based on a list of ids, then we already have a selector for it inside createTable
: https://github.com/neurosnap/robodux/blob/master/src/create-table.ts#L46
allIds is another unexpected omission. Is there some "robodux way" of preserving the ordering which came from the API I'm missing?
The end-developer is responsible for creating a separate slice to manage ordering. For example, if I fetch /users
and store the entity data inside a users
slice and I wanted to preserve the order from the API, I would also create a userIds
slice which is a createList
that stores the list of user ids. This is a departure from redux-toolkit
and something I would entertain changing given a satisfactory API.
Since it looks like modules are supposed to be self contained I don't know if it would be allowed or even desirable to have the "posts" module importing the "users" actions, it would run into circular dependency issues eventually.
As long as the slices do not listen for actions from other slices and the logic lives within the side-effect layer (e.g. sagas) then I allow it in my codebases. listifi is a "production" repo that demonstrates how I use robodux
and saga-query
and you'll see that package import other packages. The key is that the dependency tree needs to be a directed acyclic graph. In other words, posts
can import users
but users
cannot import posts
. users
should not know anything about posts
but posts
can know about users
. There are times when this does cause circular dependencies and at the point I consider it an organizational code smell and try to rethink how I'm organizing my code. I talk about it briefly in my post about scaling react/redux codebases for multiple platforms.
Could you expand on what you mean by a
byColumnIds
reducer or selector? If the goal of such a selector is to be able to find data based on a list of ids, then we already have a selector for it insidecreateTable
: https://github.com/neurosnap/robodux/blob/master/src/create-table.ts#L46
It's actually the opposite of that selector. You have a value for a given column and want to get an array of ids from rows where the column matches this value. Some use cases:
posts
by type
(indexing approach)state.posts
{
byId: {
1: { type: 'foo', title: 'Hello foo' },
2: { type: 'bar', title: 'Hello bar' },
3: { type: 'foo', title: 'Goodbye foo' }
},
byTypeIds: {
foo: [1, 3],
bar: [2]
}
}
const [1, 3] = useSelector(state => state.posts.byTypeIds.foo)
posts
by title
(selector approach)const searchTerm = 'hel'
const allPosts = useSelector(state => state.posts.byId)
const [1, 2] = reduce(
allPosts,
(res, v, k) => v.title.toLowerCase().includes(searchTerm) ? [...res, k] : res,
[]
)
The end-developer is responsible for creating a separate slice to manage ordering. For example, if I fetch /users and store the entity data inside a users slice and I wanted to preserve the order from the API, I would also create a userIds slice which is a createList that stores the list of user ids. This is a departure from redux-toolkit and something I would entertain changing given a satisfactory API.
I think I can see a higher order factory createEntity
which builds upon createTable
and friends. Then, when creating an entity you could, say createEntity({ name: 'posts', order: 'preserve | asc | desc', index: [ 'type', 'author'], search: ['title'] })
. What do you think?
users should not know anything about posts but posts can know about users.
We can't make assertions like this for APIs we don't control. For instance, the GET /users
response could have some posts
properties not present on GET /posts
. Even worse, maybe the entity you are looking for doesn't have a dedicated endpoint at all and your only escape is to build it locally by merging properties from its many appearances throughout the API responses.
It's actually the opposite of that selector.
Thanks for the clarification! For the indexing approach, are you thinking about adding a generic API to support it? I'm curious of your thoughts.
FWIW I would generally take the selector approach in my production codebases with the exception of using reselect
to memoize it.
I think I can see a higher order factory createEntity which builds upon createTable and friends. Then, when creating an entity you could, say createEntity({ name: 'posts', order: 'preserve | asc | desc', index: [ 'type', 'author'], search: ['title'] }) . What do you think?
I would be receptive to that idea! I could see it being useful with something like saga-query
as well. I'm pretty weary of building to many levels of abstraction that turns the API for robodux into a configuration object. So I'd love to see a balance between configuration and providing the tools for the end developer to leverage the API to build something for their particular needs. Any ideas on how you could make createEntity
less of a wrapper and more of a utility for composition?
We can't make assertions like this for APIs we don't control.
Agreed and I have run into this problem before. This is where trying to organize code by feature tends to breakdown and circular dependencies become an issue. What I do right now is if posts
and users
both return each other as an embedded entity in their respective API responses, then the logic that does the fetching would be in a separate package postUsers
or something like that.
One reason why I built saga-query
was to make it easier to create a generic middleware function that automatically extracts entities from API responses. This is relatively straight-forward when the API responses are structured in a consistent manner using json-api or hal.
First of all sorry for the almost a year late reply! I've decided to think a little more about this.
Looking back I can see that a memoized selector is indeed a cleaner and more flexible way to provide the indexing feature. I guess having a selector creator which takes a groupBy function and returns the relevant selector(s) should work.
Any ideas on how you could make
createEntity
less of a wrapper and more of a utility for composition?
Now that is tricky indeed. At that time I had only considered the case where you have the whole state synced locally, so it looked like all that was needed for this was one level of nesting inside the slice and some fancy selectors.
When I had to implement a paginated API I noticed the need for sorting, searching, etc. had to be handled at a higher layer, perhaps using redux-saga
and saga-query
. There must be a repeating pattern somewhere, regardless of the data source, but maybe not enough to live inside robodux
? Would it make sense to bring "general purpose-ish" selectors/factories here? I did miss the useLoader
hook as createApi
was moved out, since the loader is here and I was not using saga-query
(but it was not a major loss really).
What I do right now is if
posts
andusers
both return each other as an embedded entity in their respective API responses, then the logic that does the fetching would be in a separate packagepostUsers
or something like that.
I've adopted that but eventually the postUsers
package grew so much that I began wondering why not just merge these packages (user
, post
and postUsers
) together. After all, we find ourselves needing to select users
by posts
and vice versa more often than not. But as the same may also become true for other closely related entities, this would lead to the ultimate consolidation ino a single api
or entities
package which, idk, maybe works?
Now speaking of saga-query
, I really liked it overall and have been considering adopting it. But I scratching head trying to improve dynamic endpoints for my use case.
For instance, suppose you have an edit user page where you'd like to display a loader for saving and another, elsewhere in the screen, for deleting. And/or there's a delete button which is displayed in some sort of users
grid. And/or some other operation, I think you got the point
In this scenario it doesn't make sense to have one loader per operation type nor one loader per user but dynamic endpoints as is default to the former, while allowing the latter to also be done. Depending on the UI, it may need to select the loading status by entity id, endpoint name, both or, being wild here, some combination to trigger a full screen overlay when certain endpoints are loading, regardless of the entity id.
IMO the core issue here is that we don't have composite keys within redux, so I come empty on how to extend the loader slice in a way that benefits dynamic endpoints without cluttering it.
But as the same may also become true for other closely related entities, this would lead to the ultimate consolidation ino a single api or entities package which, idk, maybe works?
Yep, I totally agree with you there. It's a balancing act. The ultimate reason why entity packages need to mix is because of selectors
and effects
. I think of effects as the central processing unit for a react/redux application. It's where all the business logic lives and performs most of the ETL coordination. So whenever I think about creating a new package, I think about moving those two first.
One thing I've been thinking about a lot lately is moving selectors into a top-level package. I think there could be a lot of value in being able to scan all the selectors in a single file -- and it would solve a bunch of circular dependencies. And when you think about it, a selector always needs access to the entire state. You could make package-level selectors, but that only works for specific use cases. So a single entity package "owning" selectors doesn't quite make sense.
For instance, suppose you have an edit user page where you'd like to display a loader for saving and another, elsewhere in the screen, for deleting. And/or there's a delete button which is displayed in some sort of users grid. And/or some other operation, I think you got the point
Definitely an issue with the auto-generated loader pattern. I'd be happy to discuss your ideas here, but my general approach is to go manual and add another loader keyed by entity id. There's no reason why an effect can't have two loaders and when the need arises I don't hesitate to create another loader in my effect manually. I feel like it's a decent compromise, however, it's still not perfect.
I also have another issue with loaders. Since the loader action type is always the same because the payload contains the loader id, using the basic version of take
doesn't work. You'd have to use the function callback version or take all loader actions and then filter based on the payload. Not a big deal, but a somewhat more advanced usage of take
is necessary.
I've been trying to talk to Andarist about moving saga-query
into the redux-saga
org so hopefully that could gain some visibility, but it's tough. redux-saga
is a pretty stable library at this point and slowly losing its popularity. Anyway, if you want to talk more specifics relating to saga-query
feel free to open an issue or discussion there.
First of all both
robodux
andsaga-query
seem to very much have the correct opinions about state and side-effect management within react applications. Nice work!I'm considering refactoring my app to use robodux but after reading the documentation, code, article etc. I still have some questions:
Is a
byColumnIds
"index" reducer or selector considered to be within the scope of robodux? If so I can see if I can send a PRallIds
is another unexpected omission. Is there some "robodux way" of preserving the ordering which came from the API I'm missing?Speaking of APIs... I've been feeding my "byId" reducers with the output from normalizr and each slice figures out it is time to update its state if it finds a relevant entity for itself:
Why? Multiple entities come in different forms and shape, from multiple endpoints, etc. and the declarative approach of normalizr makes parsing these entities a lot easier and less error prone than writing procedural code to deal with each case individually.
So having normalizr coupled with the api package and sending those generic
ADD_DATA
actions makes sense to me.From what I understood robodux expects me to be able to do a
GET /users
to fillstate.users
rather than hidrate my state withcommenters
andauthors
fromGET /posts
.Since it looks like modules are supposed to be self contained I don't know if it would be allowed or even desirable to have the "posts" module importing the "users" actions, it would run into circular dependency issues eventually.