[changed] Don't update scroll if only query has changed

Previously, the only way to opt out of scroll updates for a route would be by using `ignoreScrollBehavior`.
This, however, made it hard to implement arguably the most common use case: resetting scroll when `params` change and preserving it when only `query` changes.

This commit completely disables scroll updates when only `query` has changed.
This provides a reasonable default behavior and leaves `ignoreScrollBehavior` for more complicated cases.

If you'd rather keep the old behavior and reset scroll on query changes, you can either promote `query` variables to be route `params` or reset scroll position yourself in response to `query` changes in route handler's `componentDidUpdate`.

Fixes #432, #439

Author: gaearon

modules/__tests__/Router-test.js

@@ -353,7 +353,8 @@ describe('Router.run', function () {
           <Route handler={Foo} path='/feed' />
           <Route handler={Foo} path='/discover' />
         </Route>
-        <Route path='/search' handler={Foo} ignoreScrollBehavior />
+        <Route path='/search/:q' handler={Foo} ignoreScrollBehavior />
+        <Route path='/users/:id/posts' handler={Foo} />
         <Route path='/about' handler={Foo} />
       </Route>
     );
@@ -406,25 +407,50 @@ describe('Router.run', function () {
       });
 
       it('calls updateScroll when no ancestors ignore scroll although source and target do', function () {
-        TestLocation.push('/search');
+        TestLocation.push('/search/foo');
         expect(didUpdateScroll).toBe(true);
       });
 
-      it('calls updateScroll when source is same as target and does not ignore scroll', function () {
-        TestLocation.push('/about?page=2');
+      it('calls updateScroll when route does not ignore scroll and only params change', function () {
+        TestLocation.replace('/users/3/posts');
+        didUpdateScroll = false;
+
+        TestLocation.push('/users/5/posts');
+        expect(didUpdateScroll).toBe(true);
+      });
+
+      it('calls updateScroll when route does not ignore scroll and both params and query change', function () {
+        TestLocation.replace('/users/3/posts');
+        didUpdateScroll = false;
+
+        TestLocation.push('/users/5/posts?page=2');
         expect(didUpdateScroll).toBe(true);
       });
 
+      it('does not call updateScroll when route does not ignore scroll but only query changes', function () {
+        TestLocation.replace('/users/3/posts');
+        didUpdateScroll = false;
+
+        TestLocation.push('/users/3/posts?page=2');
+        expect(didUpdateScroll).toBe(false);
+      });
+
       it('does not call updateScroll when common ancestor ignores scroll', function () {
         TestLocation.push('/discover');
         expect(didUpdateScroll).toBe(false);
       });
 
-      it('does not call updateScroll when source is same as target and ignores scroll', function () {
-        TestLocation.push('/search');
+      it('does not call updateScroll when route ignores scroll', function () {
+        TestLocation.replace('/search/foo');
         didUpdateScroll = false;
 
-        TestLocation.push('/search?q=test');
+        TestLocation.push('/search/bar');
+        expect(didUpdateScroll).toBe(false);
+
+        TestLocation.replace('/search/bar?safe=0');
+        expect(didUpdateScroll).toBe(false);
+
+        TestLocation.replace('/search/whatever');
         expect(didUpdateScroll).toBe(false);
       });
     });

modules/mixins/Scrolling.js

@@ -1,15 +1,22 @@
 var invariant = require('react/lib/invariant');
 var canUseDOM = require('react/lib/ExecutionEnvironment').canUseDOM;
 var getWindowScrollPosition = require('../utils/getWindowScrollPosition');
+var Path = require('../utils/Path');
 
 function shouldUpdateScroll(state, prevState) {
   if (!prevState) {
     return true;
   }
 
+  var path = state.path;
   var routes = state.routes;
+  var prevPath = prevState.path;
   var prevRoutes = prevState.routes;
 
+  if (Path.withoutQuery(path) === Path.withoutQuery(prevPath)) {
+    return false;
+  }
+
   var sharedAncestorRoutes = routes.filter(function (route) {
     return prevRoutes.indexOf(route) !== -1;
   });