redux-crud-store
redux-crud-store copied to clipboard
Return structure of selectRecord suboptimal
Hi,
is there a reason selectRecord()
returns either a Selection
or the server response? There could be collisions, if the server response has e.g. the field isLoading
or error
.
I think the return value could be simplified by not returning sel.data
but the whole object. In this case, you always know what you're dealing with.
And last, I don't understand why the error
field contains an Error object when in loading state. Shouldn't the error be empty then?
In summary, I'd propose an object structure like this:
return {
isLoading: boolean,
needsFetch: boolean,
error?: Error,
data?: T
}
Thanks for this issue! The updates to the Flow spec of the return type sounds like a positive change.
I'm more hesitant about removing data envelopes from the lib. In hindsight, it would certainly simplify the code, but it would push a bit more work onto the users. One goal of this library is that if you have a reasonably normal API (including APIs with and without data envelopes), this should work without too much configuration. Anything crazier and you're expected to implement the parsing logic using a custom ApiClient or something else.
Anyways, if you write this up as a pull request I'm happy to review and merge.
I don't see how this would complicate things, rather than making them easier. If you can be sure that you've got always a isLoading
, and, if not loading, a data
field, and that there's only an error
field in case of an error, it appears more transparent to me.
Since this would break backwards compatibility, I assume it's more a feature for a new major version. Maybe it's a good opportunity to bring https://github.com/devvmh/redux-crud-store/issues/38 up-to-date and add this feature request.
I agree it's a major simplification to the logic of the lib.
But for users, it pushes just a little bit more work onto them. I'd rather see a PR that improves the output of the selectors while maintaining its ability to "parse" data envelopes.