Dynalon / reactive-state-react-example

A full React Counter/Todo application using reactive-state as a Container plus RxJS instead of Redux
6 stars 0 forks source link

example for using inputProps in connect, like ownProps for redux #1

Closed cramatt closed 5 years ago

cramatt commented 5 years ago

Hello, fantastic library - we are exploring as replacement for Redux app powered with RXJS, seems like the perfect fit! One use case for connect we are not sure to implement in reactive-state is ownprops. We see inputProps available as behaviorSubject in connect, but we are having trouble using this effectively. Here is the example from redux docs: https://react-redux.js.org/api/connect#ownprops

const mapStateToProps = state => ({ todos: state.todos.map(todo => todo.id) })

const mapStateToProps = (state, ownProps) => ({
  todo: state.todos[ownProps.id]
})

We are very close in reactive-state, but are not sure how to connect inputProps to store.watch.


export ConnectedTodoList = connect(
  TodoList,
  (store) => {
    const props = store.watch((state) => ({
      todos: map(state.todos, 'uuid')
    }));
    return {
      actionMap: {},
      props
    };
  }
);
export ConnectedTodo = connect(
  Todo,
  (store, inputProps) => {
    const props = store.watch((state) => {
      return {
        todo: state.todos[1] // @todo use inputProps.id here
      };
    });
    return {
      actionMap: {},
      props
    };
  }
);

Thanks in advance for your help!

cramatt commented 5 years ago

Made some progress here, got it working but have a question regarding performance:

export ConnectedTodo = connect(
  Todo,
  (store, inputProps) => {
    const props = zip(
      inputProps.pipe(map(e => e.id)),
      store.watch((state) => state.todos)
    ).pipe(map(([id, todos]) => ({
      todo: todos[id]
    })));
    return {
      actionMap: {},
      props
    };
  }
);

The above code works, but my understanding from the docs is that this will run more often than it needs to because store.watch((state) => state.todos) will run on any todo update.

So, I got this working too:

export ConnectedTodo = connect(
  Todo,
  (store, inputProps) => {
    const props = inputProps.pipe(
      map(e => e.id),
      switchMap((id) => store.watch((state) => ({
        todo: state.todos[id]
      })))
    );
    return {
      actionMap: {},
      props
    };
  }
);

It seems the second is most performant... thoughts? Is this correct? Is there a better way to approach all of this? Thanks!

Dynalon commented 5 years ago

Good catch, I didn't document the inputProps anywhere and it is not used in the example app. There is a test for reference though here which might help: https://github.com/Dynalon/reactive-state/blob/master/test/test_react_connect.tsx#L229

As for you case: If I understand correctly, you want to take a id from the inputProps of the connected component and map it to a Todo that matches this id. Your second approach looks overly complicated using switchMap so lets discard it. Your first approach using zip is probably not correct (will only update if both, the id and the store update). You were probably looking for combineLatest rather then zip:

export const ConnectedTodo = connect(Todo, (store, inputProps) => {
    const todos = store.watch(state => state.todos);
    const id = inputProps.pipe(map(e => e.id));
    const props = combineLatest(todos, id).pipe(
        map(([todos, id]) => ({ todo: todos[id] }))
    );
    return {
        actionMap: {},
        props
    }
});

I don't see any performance implications here. When id updates or the todos update, then the selected todo is also updated (which will trigger a rerender). I could only see performance issues if you parent component updates id for some reason too often.

I wouldn't worry about performance in React with reactive-state (or redux) until you really identify a bottleneck with the profiler. If this at some point is the case there are very simple mitigation solutions (again, don't use them before you have a profiler report that backs you up, until then it is premature optimisation):

  1. You could use the distinctUntilChanged operator on id or props - for the later you would need to implement a shallow equal comparison
  2. You could make the component that is connected (the "dumb") component a PureComponent or implement shouldComponentUpdate yourself. If you can't change the dumb component, you could create a wrapper component, which in turn you can connect. Depends on what is the actuall bottleneck (i.e. causes a lot of rerenders).