Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
jmarceli opened this issue Jul 29, 2016 · 4 comments
Closed

Comments

@jmarceli
Copy link
Contributor

jmarceli commented Jul 29, 2016

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
Copy link
Collaborator

tomkis commented Jul 29, 2016

Interesting, I was also thinking about such a feature earlier. @slorber was also thinking about something similar (slorber/scalable-frontend-with-elm-or-redux#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
Copy link

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
Copy link
Contributor Author

jmarceli commented Jul 29, 2016

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.

@jmarceli jmarceli changed the title React-router change route without unmounting component React-router @@router/LOCATION_CHANGE available in subcomponents Updaters Jul 29, 2016
@tomkis
Copy link
Collaborator

tomkis commented Aug 1, 2016

@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.

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

No branches or pull requests

3 participants