Skip to content
This repository has been archived by the owner on Jan 27, 2021. It is now read-only.

[react-redux-subspace] Add string to prop types of mapState prop #54

Merged
merged 1 commit into from
Oct 30, 2017
Merged

[react-redux-subspace] Add string to prop types of mapState prop #54

merged 1 commit into from
Oct 30, 2017

Conversation

mradionov
Copy link
Contributor

According to the docs mapState may receive a string, but when I use <SubspaceProvider mapState="someString" /> React throws a warning about invalid prop type.

This PR extends prop types for the <SubspaceProvider> component to expect either string or function.

@mpeyper
Copy link
Contributor

mpeyper commented Oct 27, 2017

Of course! Good pick up.

I'm away from home until tomorrow night, so I'll merge this and push a new version then, unless on of the others get to it first.

@mpeyper mpeyper merged commit ef18195 into ioof-holdings:master Oct 30, 2017
@mradionov
Copy link
Contributor Author

mradionov commented Oct 30, 2017

@mpeyper btw, thank you for the great lib. My project got to the point when I had to use same components with their Redux infrastructure (reducer, actions, selectors) multiple times in the project. By default in Redux everything is so tightly coupled and it's pretty difficult do achieve some kind of reuse without code duplication. So I had to start thinking about how to make it work, so I've identified the problems which were stopping me from reuse:

  1. reducer unique position in state tree and it's unique name
  2. selectors which depended on the reducer position in state tree
  3. actions got dispatched for all mounted instances

I've even started working on some kind of proof of concept with higher-order reducers and action namespacing, when I saw your posts in this React issue. I was very happy to find a library which has everything I was thinking about of doing by myself and it has even more than I needed, thank you for spreading the word. Your implementation is very nice and pure, I am very grateful for your efforts of putting this library together. Good job!

@mpeyper
Copy link
Contributor

mpeyper commented Oct 31, 2017

😊 Thanks @mradionov. I'm glad you find it useful.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants