Skip to content

Conversation

@jcperez-ch
Copy link
Contributor

As promised, the squash function described in #158

config = {
  squash: (action: Action, state: State, history: StateWithHistory<State>) => boolean
}
  • a config entry that allow users to synchronize the _latestUnfiltered state with the present state
  • When a Filtered&Squashed action is dispatched:
    • _latestUnfiltered will become the most recent present
  • When a Not-Filtered&Squashed action is dispatched:
    • _latestUnfiltered will become the latest present just before the history insertion.
  • When a Filtered&Not-Squashed action is dispatched: same as before.
  • When a Not-Filtered&Not-Squashed action is dispatched: same as before.

@davidroeca
Copy link
Collaborator

davidroeca commented May 9, 2017

This squash approach is very close to the filtering hack, which is why I was hesitant to implement it in this way. It has some unnecessary complexity for the end user. It looks to me like the boolean would need to infer the previous 'squash' state through some form of state stored in the reducer wrapped with undoable.

e.g. if I want to squash all 'CHANGE' actions and all 'DRAG' actions, how would I know to squash the following set of actions 'CHANGE', 'CHANGE', 'CHANGE', 'DRAG', 'DRAG', 'DRAG' into two undos? My squash function would likely get more complex than I'd personally want, or I'd somehow have to infer that the user has stopped 'CHANGE'ing and switched to 'DRAG'ging.

@jcperez-ch
Copy link
Contributor Author

@davidroeca Would it make sense to add a _latestAction to the history: StateWithHistory<State> state? so we could leverage the 3rd argument of the Squash function.

I didn't think in squashing/filtering in stream chunks like the example you just pointed.

I agree, the squash and filter actions would become a bit more overwhelmed.

I just tried and solved my use case, where I just filter and squash on browser/history pushes, so it creates history entries of the latest (not the initial as before) state before a route transition.

@davidroeca
Copy link
Collaborator

_latestAction could definitely work. It would be a little annoying handling the createHistory, and the undos/redos, etc. There will likely be weird edge cases between those, so maybe it should just be set to null there and handled appropriately in the squash function.

@jcperez-ch
Copy link
Contributor Author

Also, I feel like the name of the function squash doesn't match with the actual implementation.

If you guys decide to keep the functionality, then it should be renamed to syncFilter or something like that, I am open to ideas...

@davidroeca davidroeca mentioned this pull request May 10, 2017
@jcperez-ch
Copy link
Contributor Author

Guys, I know this PR has squash on the title, but it is actually a synchronization of the _latestUnfiltered to the present states. I believe this is a useful use case, and I am not sure why it could actually mean a hack.

Let me know if we keep going on preserving this effort.

@davidroeca
Copy link
Collaborator

Agreed, though the added function seems redundant with filtering. If filtering and groupby don't elegantly handle the use case, maybe a config boolean for filtering behavior as you mentioned in #158 might make more sense?

@jcperez-ch
Copy link
Contributor Author

@davidroeca right. we could safely close this PR and I can work on the syncFilter flag instead.

The spec would simply be:

  • When syncFilters is true and a unfiltered action is dispatched: The _latestUnfiltered state will change with the value of the present state
  • When syncFilters is false and a unfiltered action is dispatched: The _latestUnfiltered state will remain as before.
  • When a filtered action is dispatched: The syncFilters flag is irrelevant.

All good @davidroeca ?

@davidroeca
Copy link
Collaborator

Sounds good to me 👌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants