Managing child component state

[kuon]

I'm trying your library for a new project, and I just hit a small problem.

Let's consider the following structure:

<SplitView id="my-splitview">
  <SplitViewPane>
    something
  </SplitViewPane>

  <SplitViewPane>
    something else
  </SplitViewPane>
</SplitView>

I'd like the SplitView component to manage the state of it's children, and I'd like the children to be able to read props and dispatch actions in the context of the SplitView.

For example, this is what I have to do now:

function render(props) {

  let items = []

  props.children.forEach((child, i) => {
    items.push(React.cloneElement(child, {
      key: i,
      horizontal: props.horizontal,
      collapsed: props.panes[i].collapsed,
      height: props.panes[i].height,
      width: props.panes[i].width,
      toggleCollapsed: props.toggleCollapsed 
    }))

  })

  return (
    <div>
      { items }
    </div>
  )
}



export default
local({
  key: (props) => props.id,
  createStore: (props) => {
    let panes = props.children.map((child) => child.props)
    return createStore(reduce, { panes })
  },
  mapStateToProps: (state, props) => {
    return {
      panes: state.panes
    }
  },
  mapDispatchToProps: (dispatch) =>({
    toggleCollapsed: (index) => dispatch(toggleCollapsed(index))
  })
})(render)

In this example, the toggleCollapsed action is passed to the child, along with the state.

What I'd rather do, is to only pass an index propery to my children panes, and have a local in my children looking like this:

// in child

function render(props)  {  
return (
    <div className="this-is-split-view-pane" onClick={props.toggleCollapsed}>
    { props.children }
    </div>
  )
}

export default
local({
  key:  // parent key, this is inherited somehow
  createStore: // this is inherited somehow
  mapStateToProps: (state, props) => {
    return state.panes[props.index] // here I can access the PARENT state
  },
  mapDispatchToProps: (dispatch) =>({
    // this dispatch to the parent store
    toggleCollapsed: (index) => dispatch(toggleCollapsed(index))
  })
})(render)

This code might have typos, I tried to simplify my real case into an understandable example.

But I hope the paradigm is simple enough.

[gcazaciuc]

I'm not sure exactly what you are trying to accomplish and why you cannot just give the child the toggleCollapsed prop.
Even so in your example with a local wrapping the child component you already can pass the pane state from the parent to the child, so basically you can just give it the piece of state the child needs as props.
For example in mapStateToProps above of the child in the props you would have props.pane passed from the parent.

Other ways:

1. Just drop the 'local' on the parent component and have each child wrapped in local. They'll each get their own store and dispatch. Each one could have the local HOC defined as:

export default
local({
  key:  (props) => props.childId, // Anything that is globally unique will do
  createStore:  (props) => {
      return createStore(childReducer, { collapsed: false, width: 100, height: 100 }) , 
  },
  mapDispatchToProps: (dispatch) =>({
    // this dispatch to the parent store
    toggleCollapsed: () => dispatch(toggleCollapsed())
  })
})(render)

You don't event need the index in this case in toggleCollapsed. Each child will have their own state reducer doesn't need to know which child is updating.
The children state can be read as any time from anywhere because the state is persisted on teh global store at state.local.childId .

1. You can pass the parent dispatch as prop and having it accesible in mapDispatchToProps:( dispatch, ownProps) on the ownProps and dispatch using it.
2. You connect the children to the global state directly using 'connect' instead of local, do the dispatches as usual and have the parent reducer handle the actions and update the parent local state.
   You can read in the connected child the parent state in mapStateToProps by accessing state.local.parentComponentKey.

There are lots of ways to access everything from anywhere. However I would say to pass the pieces of parent state that are absolutely needed by the child,
and handle parent/child communication via plain old callback.
The same as it's not a good idea to have lots of staetfull components in an app it's not a great idea also to connect everything in 'local' when you can get away more easily with a parent holding the state for multiple components.

[kuon]

The problem with "plain old callback" is that it's a pain to manage when it's deep down in the hierarchy.

For example, let's consider the following tree:

<A id="first">
  <B>
    <C>
    </C>
  </B>
</A>
<A id="second">
  <C>
  </C>
</A>

From my C component, I'd like to dispatch actions that will be handled by the reducer of the A component. I also want my C component to get props from the state of A.

The current react-redux connect and Provider does that in a way if I do something like:

<Provider>
<A>
  <B>
    <C>
    </C>
    <C>
    </C>
  </B>
</A>
</Provider>
<Provider>
<A>
  <C>
  </C>
</A>
</Provider>

But the problem with that is that there is no communication between the two stores as there are two Provider.

I cannot have a local on each child component, as their state is coupled (for the splitview example, if I grow one pane, the other should shrink).

[gcazaciuc]

Makes sense. We could implement something along these lines:
Components having the same key use the same store. In the example below it would involve somehow
that SplitView id would get passed down somehow as prop to the child component(in this case a prop with the name parentSplitViewId ).

local({
    key: (props) => props.id, // return 'mysplitview'
    createStore: (props) .....
})(SplitView)

And then to share the store in a child component:

local({
key: (props) => props.parentSplitViewId //  returns same key as parent 'mysplitview'
mapStateToProps: (state, ownProps) // state is the whole parent state
mapDispatchToProps: (dispatch) // dispatch is the parent state dispath

Thoughts ?

[gcazaciuc]

Just to make it clear, what i proposed above is not yet implemented in redux-fractal. But it shouldn't be hard to do so.

[kuon]

Yes, this is what I meant.

How do you plan to have "parentSplitViewId" passed deep down without having to specify it or pass it manually? Can we rely on react's context for this?

It would make sense to have something along there lines:

In parent component:

local({
class: "split-view",
key: (props) => props.id, // a unique key
...

And in child components

local({
class: "split-view",
...

The child component has no key, but a class, and it means go up through the component hierarchy until I find a parent component with this class, when found, its state will get passed to mapStateToProps and mapDispatchToProps dispatch would be bound to this parent component.

This would also allow having multiple local on a child component. If the child component must dispatch/receive props from different parents. For example, click a child item in a collection could dispatch a select action to the collection parent, and a scroll me to visible action to the scroll view wrapping the collection. This kind of dispatch is better handled at the item level, because if we dispatch select to the collection and then try to dispatch scroll that item to visible from the collection, the collection has to go down to the item to "measure it and see if it's visible".

[gcazaciuc]

There are a few design decisions that I would like to stick to:

1. Let's not bloat the HOC API un extra settings that would only make it confusing. I wourld like to keep the API in the current form and seek to accomodate as much use cases as possible without it becoming bloated
2. Remove the 'magic' in handling state and prefer explicitness and predictability.

That said, i don't think is a good idea to add extra configs or to enable behaviour like connecting to the first parent store because it makes things error prone and more importantly unpredictable.

There are however a few improvement we could make around the existing API:

1. Have all callback style configs, createStore, key, 'persist' accept besides props the current component context as param. This would enable a higher level component to put data on context that might be used in the process of generating the key, creating the store, whether or not to persist comp state etc. Eg if you would really want not to pass the parentSplitViewId as prop you could read it from the context no matter how deep in the hierarchy after the parent component puts it there
2. Have components returning the same value for key share the same store.

Regarding inter-component communication, eg a child dispatching a select and scroll me events that are caught in 2 different stores( let's say CompA and CompB local stores) that can already be achieved:

1. child dispatches both actions
2. CompA has the filterGlobalAction(action) that checks if action type is 'select' and returns true in that case.
3. CompB has the filterGlobalAction(action) that checks if action type is 'scroll me' and returns true in that case.

As you can imagine the checks can get as elaborate as you want( can check action.meta, payload etc).
That should cover just about anything.

[kuon]

I think the two changes you plan will be enough for me to do what I want.

[gcazaciuc]

Implemented in 1.3 as discussed above. Docs updated( check the recipes section)

[kuon]

Your changes are good and my first test shows it works.

One question about your change: is it safe to chain local?

For example:

import actionsA from "./ducks/a"

local({
  key: (props, context) => context.parentAId,
  mapDispatchToProps: (dispatch) => {helloA: dispatch({type:"HELLO"})},
  mapStateToProps: (state, props) => {somethingFromA: state.somethingFromA}
})(
local({
  key: (props, context) => context.parentBId,
  mapDispatchToProps: (dispatch) => {helloB: dispatch({type:"HELLO"})},
  mapStateToProps: (state, props) => {somethingFromB: state.somethingFromB}
}))(MyComponent)

This would cover all composition use case I can think of.

[gcazaciuc]

Yes, i cannot think of a case where it wouldn't be safe. Each HOC is independent and don't affect each other.

[gcazaciuc]

I will also add some unit tests in the coming days to prove this use case works just fine and ensure it stays that way as we continue making changes/fixing bugs.