salsita / prism

React / Redux action composition made simple http://salsita.github.io/prism/
495 stars 24 forks source link

React-router @@router/LOCATION_CHANGE available in subcomponents Updaters #40

Closed jmarceli closed 8 years ago

jmarceli commented 8 years ago

Use componentWillReceiveProps method

see: example below

--- ORIGINAL QUESTION One of the issues during react-router integration is detecting route changes which doesn't trigger component unmounting. Example: I've got routes: orders/1, orders/2, etc. (defined with orders/:id in router) All of them uses same OrderForm subcomponent so if I switch between them the only action triggered is @@router/LOCATION_CHANGE (triggered by react-router). Unfortunately OrderForm component is nested inside OrderPage component so it doesn't receive that action (because it is not prefixed properly). My current solution for that is to propagate @@router/LOCATION_CHANGE action through all of the updaters with code like the following:

// this is Updater code
  .case('@@router/LOCATION_CHANGE', (model, action) => (
    {
      ...model,
      login: loginUpdater(model.loginPage, action),
      lostPassword: lostPasswordUpdater(model.lostPasswordPage, action),
      home: homeUpdater(model.homePage, action),
      order: orderUpdater(model.orderPage, action),
    }
  )).toReducer();

Good part is that it works and I can handle @@router/LOCATION_CHANGE in subcomponents but I doesn't look nice and requires such code for every level of nesting. Does anyone has any better solution? Setting @@ as global prefix would be nice, so all actions prefixed with @@ will be available for all elm components. Even better option would be possibility to define (maybe in array) all "global" action prefixes. It will solve future possible problems with other react libraries (e.g. redux-form prefix for redux-form related actions).

tomkis commented 8 years ago

Interesting, I was also thinking about such a feature earlier. @slorber was also thinking about something similar (https://github.com/slorber/scalable-frontend-with-elm-or-redux/issues/2).

Anyway for your specific usecase, it does make sense to dispatch an action in componentWillReceiveProps when your router params change (it's pretty common actually in react-router world).

eliperelman commented 8 years ago

Also of use is the pattern laid out in redux-boilerplate by using onEnter to emit a URL change action. Doesn't cover all use cases but maybe a good majority.

jmarceli commented 8 years ago

Thanks for replies. @tomkis1 Your solution works well in my case. Example below (for future reference):

class OrderPage extends React.Component {
  componentWillReceiveProps(nextProps) {
    if (this.props.params.id !== nextProps.params.id) {
      // this will be dispatched each time the route is changed from `orders/1` to `orders/2` 
      this.props.dispatch({ type: action.Load, orderId: nextProps.params.id });
    }
  }
  render() { ... }
}
export default view(({ model, dispatch, params }) => (
  <OrderPage dispatch={dispatch} model={model} params={params} />
));

@eliperelman onEnter is nice alternative but I'm afraid that in my case it would be hard to track down all "routing" buttons and add onEnter to all of them (high probability of runtime bugs).

Anyway @tomkis1 how do you feel about idea of "global" action prefixes. I know that they should not exists in perfectly fractal (Elm) world but for ensuring compatibility with standard react/redux libraries they might be useful.

tomkis commented 8 years ago

@jmarceli I think it's something we can totally consider adding as part of the framework. However, need to figure out proper API. So far having an option to configure "global actions" seems the most sane to me.