{"action":"created","issue":{"url":"https://api.github.com/repos/ReactTraining/react-router/issues/5076","repository_url":"https://api.github.com/repos/ReactTraining/react-router","labels_url":"https://api.github.com/repos/ReactTraining/react-router/issues/5076/labels{/name}","comments_url":"https://api.github.com/repos/ReactTraining/react-router/issues/5076/comments","events_url":"https://api.github.com/repos/ReactTraining/react-router/issues/5076/events","html_url":"https://github.com/ReactTraining/react-router/issues/5076","id":226596876,"number":5076,"title":"Playing nicely with redux, mobx and pure components","user":{"login":"d-leeg","id":24990561,"avatar_url":"https://avatars2.githubusercontent.com/u/24990561?v=4","gravatar_id":"","url":"https://api.github.com/users/d-leeg","html_url":"https://github.com/d-leeg","followers_url":"https://api.github.com/users/d-leeg/followers","following_url":"https://api.github.com/users/d-leeg/following{/other_user}","gists_url":"https://api.github.com/users/d-leeg/gists{/gist_id}","starred_url":"https://api.github.com/users/d-leeg/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/d-leeg/subscriptions","organizations_url":"https://api.github.com/users/d-leeg/orgs","repos_url":"https://api.github.com/users/d-leeg/repos","events_url":"https://api.github.com/users/d-leeg/events{/privacy}","received_events_url":"https://api.github.com/users/d-leeg/received_events","type":"User","site_admin":false},"labels":[],"state":"closed","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":11,"created_at":"2017-05-05T14:48:53Z","updated_at":"2017-11-03T20:25:31Z","closed_at":"2017-07-12T03:50:28Z","author_association":"NONE","body":"I would like to open a discussion concerning the way react-router plays together with `PureComponent` and libraries like `redux` and `mobx`.\r\n\r\nThis topic was briefly touched upon in #4680, but I believe that due to the fact that it influences a huge number of developers, and that the workaround suggested there leaves a lot to be desired; a more thorough discussion is warranted.\r\n\r\nI wish to start this discussion here by considering 4 points:\r\n1. Recap of the problem.\r\n2. Arguing why the solution suggested in the [guidelines](https://github.com/ReactTraining/react-router/blob/master/packages/react-router/docs/guides/blocked-updates.md) is far from optimal.\r\n3. Analysis of the causes of the problem.\r\n4. Suggested solution.\r\n\r\nTo recap, the heart of the problem is that `react-router` uses the `context` to pass down `location` and `match` information. However, changes to this information are\r\nblocked, and not propagated down any sub-tree of the components tree that is rooted at a pure component that has no interest in the `react-router` `context`.\r\nObviously, such blocking completely breaks down the functionality of components such as `Route` in the sub-tree.\r\n\r\nUnfortunately, blocking components are rife: either due to explicit use of `React.PureComponent`, or by using libraries such as `react-redux` and `mobx`; for example, every `react-redux` component created using a regular call to `connect()` is pure. As a result, the solution suggested in the [guidelines](https://github.com/ReactTraining/react-router/blob/master/packages/react-router/docs/guides/blocked-updates.md), of punching a hole in every blocking ancestor by obtaining the location and passing it as a prop, is far from being ideal, both from a practical point of view, and from a more conceptual point of view.\r\n\r\nMore specifically, it suffers from at least the following drawbacks:\r\n1. Punching the holes must be done explicitly (i.e., it is not automatic and transparent).\r\n  \r\n2. It potentially causes many unnecessary re-renders: a punched component high up the tree will try to re-render the whole sub-tree on every location change.\r\n  \r\n3. It completely ruins modularity. This is undoubtedly the biggest issue, and for medium sized and larger projects that use `redux` it is actually a killer. The reason is that\r\n  when an affected component (such as a `Route`) is added anywhere in the component tree, all its blocking ancestor components must be punched. _In other words, a change to\r\n  one component requires a change to multiple other existing components that need not even be aware of its existence: modularity is dead_. This is a very big problem for reusing code\r\n  and, in projects where different teams are responsible for different components, it is almost intolerable. The only way to regain modularity is to have all components punched\r\n  in advance. This is clearly unacceptable as it results in a full-scale elimination of the concept of pure components.\r\n\r\n  4. Other libraries (such as `react-redux`) do not suffer from such anomalies. E.g., pure components do not block nested connected components from having access to the information they need.\r\n\r\n  5. From a conceptual point of view, why does react-router use the context? The obvious answer is in order to not have to inject unneeded props to ancestors. The proposed\r\n  solution of punching holes by passing unneeded location props to blocking components obviously negates the whole idea of using the context in the first place.\r\n\r\nI will now describe briefly what I see as the source of the problem.\r\nPlease forgive me if I am stating the obvious, or making mistakes. As Michael Jackson and Ryan Florence said in their talk at ReactJS 2016, we are all still learning how to use React.\r\n\r\nAt its core, the problem is that pure components, that do not care about the `react-router` `context`, do not pass the new `context` down when it changes.\r\nThis is definitely a conceptual problem within React, and I guess is at least part of the reason why the context is still considered experimental by the React team.\r\nHowever, this does not pose a problem if the information passed in `context` never needs to change. For example, in `react-redux` the redux store (more accurately, a pointer to it)\r\nis passed down the `context`, and this pointer never changes. Being a mutable object (observe that the _store_, unlike the _state of the store_, is mutable) one does not have to change\r\nthe pointer to the store in order to pass down the new state of the store. The latter is always accessible by calling `store.getState()`. On the other hand, `react-router` passes in the `context`\r\n_immutable data_ (namely `match` and `location` inside `route`), and there lies the problem.\r\n\r\nI suggest the following solution: have any `react-router` component, like `Router` or `Route`, that passes information down in the `context` never change that information!\r\nThus, for example, instead of passing down `route`, which needs to change as the `location` (and `match`) change, pass down a constant function `getRoute()` that returns the most up to date `route`.\r\nLet's call such a component `X`. The only missing piece now is to make sure that a client component (like `Route`) which is a descendant of `X`, will be notified whenever\r\n`X` has new `route` for it. This can be easily done using a subscription mechanism (like in `history` or `react-redux`).\r\n\r\nMore concretely, this will result, for example, in the following changes to `Router`: remove route from the child `context.router`, and add a `subscribe` and `getRoute` methods instead.\r\n\r\nThe changes to `Route` will be as follows: as for `Router`, remove `route` from the child context, and add a `subscribe` and `getRoute` methods instead. In addition,\r\nin `componentWillmount` subscribe a listener to `context.subscribe` (and unsubscribe in `componentWillUnmount`). This listener will call `context.router.getRoute()`,\r\nuse the returned ancestor `route` information to set its own `location` and `match`,\r\nand then notify all of its listeners of any changes. In addition, anywhere `context.router.route` is used, it should be replaced with `context.router.getRoute()`.\r\n\r\nAlternatively, one can get rid of `getRoute` altogether, and pass down any` route` changes to the listener callback, which will then cache that information for future use.\r\n\r\nObserve that the above solution will easily traverse any blocking components, with no ill effect, and completely transparently.\r\n\r\nThe only drawback I can see (apart from the negligible overhead) in a naive implementation of the above, is that it may cause double rendering of nested `Route` components that are not blocked:. I.e., when the location changes, first the ancestor `Route`/`Router` will start a re-render that will cause the descendant `Route` to re-render, and then the subscription mechanism will kick in triggering another re-render when the nested `Route`'s listener method gets notified. However, This can be easily circumvented (as is usual in React) by shallowly comparing the currently cached `route.location` and `route.match` with the ones in the new `route` in the notification.\r\n\r\nHere it is guys, what is your opinion?\r\n"},"comment":{"url":"https://api.github.com/repos/ReactTraining/react-router/issues/comments/341817041","html_url":"https://github.com/ReactTraining/react-router/issues/5076#issuecomment-341817041","issue_url":"https://api.github.com/repos/ReactTraining/react-router/issues/5076","id":341817041,"user":{"login":"klis87","id":11601850,"avatar_url":"https://avatars2.githubusercontent.com/u/11601850?v=4","gravatar_id":"","url":"https://api.github.com/users/klis87","html_url":"https://github.com/klis87","followers_url":"https://api.github.com/users/klis87/followers","following_url":"https://api.github.com/users/klis87/following{/other_user}","gists_url":"https://api.github.com/users/klis87/gists{/gist_id}","starred_url":"https://api.github.com/users/klis87/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/klis87/subscriptions","organizations_url":"https://api.github.com/users/klis87/orgs","repos_url":"https://api.github.com/users/klis87/repos","events_url":"https://api.github.com/users/klis87/events{/privacy}","received_events_url":"https://api.github.com/users/klis87/received_events","type":"User","site_admin":false},"created_at":"2017-11-03T20:25:31Z","updated_at":"2017-11-03T20:25:31Z","author_association":"CONTRIBUTOR","body":"Did you consider using [react-broadcast](https://github.com/ReactTraining/react-broadcast) instead of context directly? This would solve all the issues with PureComponent blocking updates."}}
{"actor":{"display_login":"klis87"}}