ioof-holdings / redux-subspace

Build decoupled, componentized Redux apps with a single global store
https://ioof-holdings.github.io/redux-subspace/
BSD 3-Clause "New" or "Revised" License
312 stars 33 forks source link

Pass extra props down to wrapped component #22

Closed jpeyper closed 7 years ago

jpeyper commented 7 years ago

I have been using redux-subspace with react-router to isolate my pages from one another.

This has worked great as it has allowed me to develop each page as almost an entirely seperate application.

How I have this setup is as follows (simplified from my actual code)

index.js

import React from 'react'
import ReactDOM from 'react-dom'
import { createStore, combineReducers, applyMiddleware } from 'redux'
import { Provider } from 'react-redux'
import { Router, Route, browserHistory } from 'react-router'
import { syncHistoryWithStore, routerMiddleware, routerReducer as routing } from 'react-router-redux'
import { SubspaceProvider, namespaced } from 'redux-subspace'
import { SubPage1, reducer as subpage1 } from './SubPage1'
import { SubPage2, reducer as subpage2 } from './SubPage2'

const reducer = combineReducers({
    routing,
    subpage1: namespaced(subpage1, 'subpage1'),
    subpage2: namespaced(subpage2, 'subpage2'),
})
const middleware = applyMiddleware(routerMiddleware(browserHistory))
const store = createStore(reducer, middleware);
const history = syncHistoryWithStore(browserHistory, store)

const SubspacedSubPage1 = () =>
    <SubspaceProvider mapState={state => state.login} namespace='subPage1'>
        <SubPage1 />
    </SubspaceProvider>

const SubspacedSubPages2 = () =>
    <SubspaceProvider mapState={state => state.characters} namespace='subpage2'>
        <SubPage2 />
    </SubspaceProvider>

ReactDOM.render(
    <Provider store={store}>
        <Router history={history}>
            <Route path="/sub-page-1" component={SubspacedSubPage1} />
            <Route path="/sub-page-2/:someParam" component={SubspacedSubPage2} />
        </Router>
    </Provider>,
    document.getElementById('root')
)

SubPage1.js

import React from 'react';
import { connect } from 'react-redux';

const SubPage1 = () =>
    <div>
        <p>Hello From SubPage1</p>
    </div>

const mapStateToProps = (state) => {
    return {
        ...state
    }
}

export default connect(mapStateToProps)(SubPage1)

SubPage2.js

import React from 'react';
import { connect } from 'react-redux';

const SubPage2 = (someParam) =>
    <div>
        <p>Hello From SubPage2 with someParam={someParam}</p>
    </div>

const mapStateToProps = (state, ownProps) => {
    return {
        ...state
        ...ownProps.params
    }
}

export default connect(mapStateToProps)(SubPage2)

This works great, except for the someParam part. React router provides the params to the components props, which is SubspaceProvider and not the wrapped component.

I did manage to get around this with the following changes to index.js

const SubspacedSubPages2 = (props) =>
    <SubspaceProvider mapState={state => state.characters} namespace='subpage2'>
        <SubPage2 {...props}/>
    </SubspaceProvider>

However, I can also see other use cases where the parent component wants to pass params down to the underlying component, but either doesn't have access to the child to do so (like my case above where it was wrapped by another component) or it has imported a subspaced component and doesn't realise it cannot pass props (its is probably a bad design to expect props and also subspace yourself, but if you did want that its current not doable).

I propose that SubspaceProvider passes any extra props it receives down to the child component.

mpeyper commented 7 years ago

My initial reaction to this was "sure, why not", but I have been thinking about it and I don't think passing on the props makes much sense.

SubspaceProvider is designed to written in a jsx block and as such, any props can easily be provided to the children, as your workaround quite nicely shows. Implementing your suggesting might work in this case but the following snippet might behave a bit unexpectedly for someone else:

const MyAwesomeComponent = () =>
    <SubspaceProvider mapState={state => state.childState} childValue='value1'>
        <ChildComponent childValue='value2'/>
    </SubspaceProvider>

What would you expect childValue to equate to?

I have 2 thoughts to get around this:

  1. Can you move the SubspaceProvider to be around the Route instead of inside it (Route might not allow this), i.e.
    ReactDOM.render(
    <Provider store={store}>
        <Router history={history}>
            <SubspaceProvider mapState={state => state.login} namespace='subPage1'>
                <Route path="/sub-page-1" component={SubPage1} />
            </SubspaceProvider>
            <SubspaceProvider mapState={state => state.characters} namespace='subPage2'>
                <Route path="/sub-page-2/:someParam" component={SubPage2} />
            </SubspaceProvider>
        </Router>
    </Provider>,
    document.getElementById('root')
    )
  2. Perhaps we could provide a function similar to react-redux's connect function for cases where you don't want to use SubspaceProvider in a jsx block that does pass on the props, e.g.
    ReactDOM.render(
    <Provider store={store}>
        <Router history={history}>
            <Route path="/sub-page-1" component={subspaced(state => state.login, 'subPage1')(SubPage1)} />
            <Route path="/sub-page-2/:someParam" component={subspaced(state => state.characters, 'subPage2')(SubPage2)} />
        </Router>
    </Provider>,
    document.getElementById('root')

Happy to hear any comments or suggestions on this.

jpeyper commented 7 years ago

While it didn't cause any errors in the console in relation to proptypes, option 1 did not work. If I did a console.log(state) in the mapStateToProps function, it gave me the entire state tree.

Option 2 (using a HOC) seems like a good compromise for places that you just want to add in subspace to something that needs to accept props.

Although having said all that, the more I play with my little project (adding more routes and default routes that point to "home" style pages), the more my "workaround" seems to actually be good solution, as I can wrap the component once and use it in any route I like... however, option 2 could probably give the same result.