search

feathersjs-ecosystem / feathers-redux

Integrate Feathers with your Redux store
MIT License
114 stars 23 forks source link

Replace initial state of queryResult from null to proper object structure #43

Closed bsubedi26 closed 6 years ago

bsubedi26 commented 6 years ago

PR to fix issue: https://github.com/feathers-plus/feathers-redux/issues/42

eddyystop commented 6 years ago

I'll wait for this discussion to finish before merging.

bsubedi26 commented 6 years ago

@erkkaha: state[opts.queryResult] within PENDING, FULFILLED and REJECTED reducers are essentially the same data structure as queryResultDefaults because of the initial state for queryResult. If you want to see for yourself, there is an example application (within example folder) in the root of this repository.

Are you referring to removing the null within PENDING, FULFILLED and REJECTED reducers like this?

[opts.queryResult]: state[opts.queryResult] || queryResultDefaults
erkkaha commented 6 years ago

@bsubedi26 With removing nulls I was thinking taking the || null completely away, since queryResult should now always exist?

bsubedi26 commented 6 years ago

@erkkaha That definitely makes sense. Unfortunately, removing || null breaks some of the old tests (which I did not write). If you would like to take a look at the tests, you can clone this repository, npm install then npm test.

erkkaha commented 6 years ago

@bsubedi26 @eddyystop Maybe this can be merged and I can do a separate PR for removing those nulls.

eddyystop commented 6 years ago

Published as 2.1.0

eddyystop commented 6 years ago

@erkkaha Let me know if you will be looking into removing the nulls.