angular-redux / ng-redux

Angular bindings for Redux
MIT License
1.16k stars 177 forks source link

feat(connector): provide prev state slice to target (function) #171

Closed AKolodeev closed 6 years ago

AKolodeev commented 6 years ago

Hi! First of all, thank you for your work! This library is really useful.

Sometimes I pass a callback function as target and I need to compare previous state slice with a new one. I can achieve this such way:

import { pick } from 'lodash';

mergeToTarget(nextState, actions) {
  const prevState = pick(this, Object.keys(nextState));

  if (prevState.foo !== nextState.foo) {
    this.handleFooChange(nextState.foo);
  }

  Object.assign(this, nextState);
  Object.assign(this, actions);
}

But I sure it can be achieved much more easily.

In this PR I provide previous state slice as third argument for target function. For initial update I provide undefined.

Also, I fixed store reducer in connector.spec.js: because of redux initial action defaultState.baz would be undefined instead of -1.

AKolodeev commented 6 years ago

@AntJanus? Or anybody?

AntJanus commented 6 years ago

I'm a fan of this but I'm wondering if there is an equivalent in the react-redux library. My vision for ng-redux has been to provide as much parity as possible without providing many extra features.

AntJanus commented 6 years ago

Sorry for the late response on this. Work + kids have kept me busy and unable to dedicate my time to the library. I'm hoping to pick it back up after the holidays! :)

AKolodeev commented 6 years ago

React components have componentWillReceiveProps \ componentDidReceiveProps hooks which will get next props include props from mapStateToProps function and have access to previous props. In AngularJS app we can't use $onChanges hook to detect state changes caused by mapStateToTarget function. So I think, the most convenient way is provide prevState in mapStateToTarget.

Thank you for answer :)

AntJanus commented 6 years ago

@AKolodeev fantastic! I'll put it on my todo list to test this out and I'll merge it in as soon as I can.

AKolodeev commented 6 years ago

Fixed conflicts. @AntJanus will you merge it? :)