Merge pull request #263 from rwjblue/ensure-is-active-tracks-loading

.eslintrc.js

@@ -40,7 +40,8 @@ module.exports = {
         'addon/**',
         'addon-test-support/**',
         'app/**',
-        'tests/dummy/app/**'
+        'tests/dummy/app/**',
+        'tests/helpers/**'
       ],
       parserOptions: {
         sourceType: 'script'

addon/helpers/is-active.js

@@ -1,13 +1,13 @@
 import { inject as service } from '@ember/service';
 import Helper from '@ember/component/helper';
 import handleQueryParams from '../utils/handle-query-params';
+import trackActiveRoute from '../utils/track-active-route';
 
 export default class IsActiveHelper extends Helper {
   @service router;
 
   compute(_params) {
-    // ensure router.currentURL is auto-tracked
-    this.router.currentURL;
+    trackActiveRoute(this.router);
 
     let params = handleQueryParams(_params);
 

addon/utils/route-params.js

@@ -1,4 +1,5 @@
 import handleQueryParams from './handle-query-params';
+import trackActiveRoute from './track-active-route';
 
 export default class RouteParams {
   constructor(router, params) {
@@ -10,8 +11,7 @@ export default class RouteParams {
   }
 
   get isActive() {
-    // ensure router.currentURL is auto-tracked
-    this._router.currentURL;
+    trackActiveRoute(this._router);
 
     return this._router.isActive(...this._params);
   }

addon/utils/track-active-route.js

@@ -0,0 +1,10 @@
+export default function trackActiveRoute(router) {
+  // ensure we recompute anytime `router.currentURL` changes
+  router.currentURL;
+
+  // ensure we recompute whenever the `router.currentRouteName` changes
+  // this is slightly overlapping with router.currentURL but there are
+  // cases where route.currentURL doesn't change but the
+  // router.currentRouteName has (e.g. loading and error states)
+  router.currentRouteName;
+}

tests/helpers/index.js

@@ -0,0 +1,11 @@
+export function defer() {
+  let deferred = { resolve: undefined, reject: undefined };
+
+  deferred.promise = new Promise((resolve, reject) => {
+    deferred.resolve = resolve;
+    deferred.reject = reject;
+  });
+
+  return deferred;
+}
+

tests/integration/helpers/is-active-test.js

@@ -1,9 +1,12 @@
 import { computed } from '@ember/object';
 import Service from '@ember/service';
 import { module, test } from 'qunit';
-import { setupRenderingTest } from 'ember-qunit';
-import { render, settled } from '@ember/test-helpers';
+import { setupRenderingTest, setupApplicationTest } from 'ember-qunit';
+import { render, settled, visit, waitFor } from '@ember/test-helpers';
+import EmberRouter from '@ember/routing/router';
+import Route from '@ember/routing/route';
 import hbs from 'htmlbars-inline-precompile';
+import { defer } from '../../helpers';
 
 const RouterServiceMock = Service.extend({
   currentRouteName: computed('currentURL', function() {
@@ -15,32 +18,90 @@ const RouterServiceMock = Service.extend({
   }
 });
 
-module('helper:is-active', function(hooks) {
-  setupRenderingTest(hooks);
+module('helper:is-active', function() {
+  module('rendering', function(hooks) {
+    setupRenderingTest(hooks);
 
-  hooks.beforeEach(function() {
-    this.owner.register('service:router', RouterServiceMock);
+    hooks.beforeEach(function() {
+      this.owner.register('service:router', RouterServiceMock);
+    });
+
+    test('it renders and rerenders when currentURL changes', async function(assert) {
+      const router = this.owner.lookup('service:router');
+
+      router.set('currentURL', '/foo');
+      this.set('targetRoute', 'bar');
+      await render(hbs`{{is-active targetRoute}}`);
+
+      assert.strictEqual(this.element.textContent, 'false', 'is-active is not true when curren route is different from target route');
+
+      router.set('currentURL', '/bar');
+
+      await settled();
+
+      assert.strictEqual(this.element.textContent, 'true', 'is-active is true now when URL has changed');
+
+      router.set('currentURL', '/foo');
+
+      await settled();
+
+      assert.strictEqual(this.element.textContent, 'false', 'is-active is false when curren route has changed');
+    });
   });
 
-  test('it renders and rerenders when currentURL changes', async function(assert) {
-    const router = this.owner.lookup('service:router');
+  module('routing', function(hooks) {
+    setupApplicationTest(hooks);
+
+    hooks.beforeEach(function() {
+      class Router extends EmberRouter {}
+      Router.map(function() {
+        this.route('foo');
+        this.route('bar');
+      });
+
+      this.owner.register('router:main', Router);
+      this.owner.register('template:application', hbs`{{is-active 'foo'}}{{outlet}}`);
+    });
+
+    test('it updates when currentURL changes', async function(assert) {
+      await visit('/');
+
+      assert.strictEqual(this.element.textContent, 'false', 'is-active is not true when curren route is different from target route');
+
+      await visit('/foo');
+
+      assert.strictEqual(this.element.textContent, 'true', 'is-active is true now when URL has changed');
+
+      await visit('/');
+
+      assert.strictEqual(this.element.textContent, 'false', 'is-active is false when curren route has changed');
+    });
+
+    test('it renders and rerenders when the URL changes into and out of loading substate', async function(assert) {
+      let slowModelDeferred = defer();
+
+      this.owner.register('route:foo', class extends Route {
+        model() {
+          return slowModelDeferred.promise;
+        }
+      });
+      this.owner.register('template:foo-loading', hbs`<div class="loading-spinner"></div>`);
 
-    router.set('currentURL', '/foo');
-    this.set('targetRoute', 'bar');
-    await render(hbs`{{is-active targetRoute}}`);
+      await visit('/');
 
-    assert.strictEqual(this.element.textContent, 'false', 'is-active is not true when curren route is different from target route');
+      assert.strictEqual(this.element.textContent, 'false', 'precond - is-active is not true when on index route');
 
-    router.set('currentURL', '/bar');
+      let visitPromise = visit('/foo');
 
-    await settled();
+      await waitFor('.loading-spinner');
 
-    assert.strictEqual(this.element.textContent, 'true', 'is-active is true now when URL has changed');
+      assert.strictEqual(this.element.textContent, 'false', 'is-active is not true when on the loading substate');
 
-    router.set('currentURL', '/foo');
+      slowModelDeferred.resolve();
 
-    await settled();
+      await visitPromise;
 
-    assert.strictEqual(this.element.textContent, 'false', 'is-active is false when curren route has changed');
+      assert.strictEqual(this.element.textContent, 'true', 'is-active is true now that model hook has fully resolved');
+    });
   });
 });