mattkrick / redux-optimistic-ui

a reducer enhancer to enable type-agnostic optimistic updates
MIT License
693 stars 36 forks source link

fix: support nested combineReducers #20

Closed maxlapides closed 7 years ago

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.5%) to 92.857% when pulling 490b6e3edd58de3964bcfc6f87595d2d35128625 on maxlapides:master into 85b5947692b0216932c064b2dcf3be415756e8a2 on mattkrick:master.

mattkrick commented 7 years ago

LGTM, mind adding a test to make sure we don't have any regressions in future releases?

maxlapides commented 7 years ago

Okay, I wrote some unit tests...only to discover that they prove that my fix doesn't fix the problem 100%. I'll take another look and see if I can find a nice fix.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 92.308% when pulling db0d0ac1a97bce1eddc9e4ab4b826cfe5b893c3a on maxlapides:master into 85b5947692b0216932c064b2dcf3be415756e8a2 on mattkrick:master.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 92.308% when pulling db0d0ac1a97bce1eddc9e4ab4b826cfe5b893c3a on maxlapides:master into 85b5947692b0216932c064b2dcf3be415756e8a2 on mattkrick:master.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 92.308% when pulling db0d0ac1a97bce1eddc9e4ab4b826cfe5b893c3a on maxlapides:master into 85b5947692b0216932c064b2dcf3be415756e8a2 on mattkrick:master.

maxlapides commented 7 years ago

@mattkrick I think @rossman15 and I have a fix, and some tests to back it up! Can you please take a look and let us know what you think?

mattkrick commented 7 years ago

huh, neat fix, i don't quite understand it. why did removing the id cause it to error?

this LGTM. mind excluding the .editorconfig?

maxlapides commented 7 years ago

Okay, I removed .editorconfig.

Removing the id caused it to error because the same redux action needs to be used at multiple levels of the reducer "tree"

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 92.308% when pulling 794556814b9c710a992a7ebb0e19025650082e9c on maxlapides:master into 85b5947692b0216932c064b2dcf3be415756e8a2 on mattkrick:master.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 92.308% when pulling 794556814b9c710a992a7ebb0e19025650082e9c on maxlapides:master into 85b5947692b0216932c064b2dcf3be415756e8a2 on mattkrick:master.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 92.308% when pulling 794556814b9c710a992a7ebb0e19025650082e9c on maxlapides:master into 85b5947692b0216932c064b2dcf3be415756e8a2 on mattkrick:master.

maxlapides commented 7 years ago

@mattkrick do you think you'll get a chance to look at this soon? :)