[ENG-4010] Address deprecated router event (#1661)

-   Ticket: [ENG-4010]

## Purpose
- Address deprecation of router events found here: https://deprecations.emberjs.com/v3.x/#toc_deprecate-router-events

## Summary of Changes
- Remove usage of `willTransition` and `didTransition` in the app router
- Remove usage of private router API from app router
- Move logic that was handled in aforementioned `willTransition`, `didTransition` and other private router API calls into a new app-instance initializer `router` (The name is sort of awful, so any alternative name suggestions would be much appreciated 😬 )
  - Fetch server-side rendered page if a given waffle flag is not enabled
  - Scroll to top on page change, except in cases of query-param only route changes

Author: futa-ikeda

app/instance-initializers/router.ts

@@ -0,0 +1,66 @@
+import ApplicationInstance from '@ember/application/instance';
+import Transition from '@ember/routing/-private/transition';
+import RouterService from '@ember/routing/router-service';
+import Ember from 'ember';
+import Features from 'ember-feature-flags/services/features';
+
+import { Blocker } from 'ember-osf-web/services/ready';
+import transitionTargetURL from 'ember-osf-web/utils/transition-target-url';
+
+import config from 'ember-get-config';
+
+const {
+    featureFlagNames: {
+        routes: routeFlags,
+    },
+} = config;
+
+export function initialize(appInstance: ApplicationInstance): void {
+    const routerService = appInstance.lookup('service:router') as RouterService;
+    let readyBlocker = null as Blocker | null;
+
+    routerService.on('routeWillChange', (transition: Transition) => {
+        const ready = appInstance.lookup('service:ready');
+        const features = appInstance.lookup('service:features') as Features;
+        if (!readyBlocker || readyBlocker.isDone()) {
+            readyBlocker = ready.getBlocker();
+        }
+
+        const isInitialTransition = transition.sequence === 0;
+        if (!isInitialTransition) {
+            const flag = routeFlags[transition.targetName];
+            if (flag && !features.isEnabled(flag)) {
+                if (!Ember.testing) {
+                    try {
+                        window.location.assign(transitionTargetURL(transition));
+                    } catch (e) {
+                        window.location.reload();
+                    }
+                }
+                transition.abort();
+            }
+        }
+
+        routerService._super(transition);
+    });
+
+    routerService.on('routeDidChange', (transition: Transition) => {
+        const currentUser = appInstance.lookup('service:current-user');
+        const statusMessages = appInstance.lookup('service:status-messages');
+        currentUser.checkShowTosConsentBanner();
+        statusMessages.updateMessages();
+        if (!transition.queryParamsOnly) {
+            const { rootElement: rootElementSelector } = appInstance as any;
+            const rootElement = document.querySelector(rootElementSelector);
+            rootElement.scrollIntoView();
+        }
+
+        if (readyBlocker && !readyBlocker.isDone()) {
+            readyBlocker.done();
+        }
+    });
+}
+
+export default {
+    initialize,
+};

app/router.ts

@@ -1,96 +1,17 @@
-import { getOwner } from '@ember/application';
-import Transition from '@ember/routing/-private/transition';
 import EmberRouter from '@ember/routing/router';
-import { inject as service } from '@ember/service';
-import Ember from 'ember';
 import config from 'ember-get-config';
 
-import { Blocker } from 'ember-osf-web/services/ready';
-import transitionTargetURL from 'ember-osf-web/utils/transition-target-url';
-
 const {
     engines: {
         collections,
         registries,
     },
-    featureFlagNames: {
-        routes: routeFlags,
-    },
 } = config;
 
-/* eslint-disable ember/no-get */
 const Router = EmberRouter.extend({
-    currentUser: service('current-user'),
-    features: service('features'),
-    statusMessages: service('status-messages'),
-    ready: service('ready'),
-
-    // eslint-disable-next-line ember/avoid-leaking-state-in-ember-objects
-    readyBlocker: null as Blocker | null,
     location: config.locationType,
     rootURL: config.rootURL,
-    shouldScrollTop: true,
-
-    willTransition(oldInfo: any, newInfo: any, transition: { targetName: string }) {
-        if (!this.readyBlocker || this.readyBlocker.isDone()) {
-            this.readyBlocker = this.get('ready').getBlocker();
-        }
-
-        this._super(oldInfo, newInfo, transition);
-    },
-
-    didTransition(...args: any[]) {
-        this._super(...args);
-
-        this.get('currentUser').checkShowTosConsentBanner();
-        this.get('statusMessages').updateMessages();
-
-        if (this.shouldScrollTop) {
-            const { application: { rootElement: rootElementSelector } } = getOwner(this);
-            const rootElement = document.querySelector(rootElementSelector);
-            rootElement.scrollIntoView();
-        }
-
-        if (this.readyBlocker && !this.readyBlocker.isDone()) {
-            this.readyBlocker.done();
-        }
-    },
-
-    _doTransition(...args: any[]) {
-        const transition = this._super(...args);
-        return this._beforeTransition(transition);
-    },
-
-    _doURLTransition(...args: any[]) {
-        const transition = this._super(...args);
-        return this._beforeTransition(transition);
-    },
-
-    _beforeTransition(transition: Transition) {
-        // Don't snap the page to the top if it's just a query param change
-        // IE registries, preprints, collections, etc
-        // There doesn't appear to be a good way to access the transition
-        // inside of didTransition, so the state is just plucked here for future reference.
-        this.shouldScrollTop = !transition.queryParamsOnly;
-
-        const isInitialTransition = transition.sequence === 0;
-        if (!isInitialTransition) {
-            const flag = routeFlags[transition.targetName];
-            if (flag && !this.get('features').isEnabled(flag)) {
-                if (!Ember.testing) {
-                    try {
-                        window.location.assign(transitionTargetURL(transition));
-                    } catch (e) {
-                        window.location.reload();
-                    }
-                }
-                transition.abort();
-            }
-        }
-        return transition;
-    },
 });
-/* eslint-enable ember/no-get */
 
 /* eslint-disable array-callback-return */
 

config/deprecation-workflow.js

@@ -4,7 +4,6 @@ self.deprecationWorkflow.config = {
         { handler: 'silence', matchId: 'ember-inflector.globals' },
         { handler: 'silence', matchId: 'ember-metal.get-with-default' },
         { handler: 'silence', matchId: 'computed-property.volatile' },
-        { handler: 'silence', matchId: 'deprecate-router-events' },
         { handler: 'silence', matchId: 'implicit-injections' },
         { handler: 'silence', matchId: 'manager-capabilities.modifiers-3-13' },
         { handler: 'silence', matchId: 'this-property-fallback' },

handbook-docs/troubleshooting/template.md

@@ -52,6 +52,7 @@ parameter. You just need to
 
 Then, either in `beforeEach` or in the particular problematic test:
 
+`this.owner.unregister('service:router');`
 `this.owner.register('service:router', OsfLinkRouterStub);`
 
 ## Buttons in modals aren't tracking analytics events

tests/acceptance/resolve-guid-test.ts

@@ -1,13 +1,12 @@
 import Transition from '@ember/routing/-private/transition';
 import Service from '@ember/service';
 import { currentRouteName, currentURL, settled } from '@ember/test-helpers';
-import { getContext } from '@ember/test-helpers/setup-context';
 import { setupMirage } from 'ember-cli-mirage/test-support';
 import config from 'ember-get-config';
 import { setupApplicationTest } from 'ember-qunit';
 import { TestContext } from 'ember-test-helpers';
 import { module, test } from 'qunit';
-import sinon, { SinonStub } from 'sinon';
+import sinon from 'sinon';
 
 import { Permission } from 'ember-osf-web/models/osf-model';
 import { currentURL as currentLocationURL, visit } from 'ember-osf-web/tests/helpers';
@@ -23,16 +22,15 @@ const KeenStub = Service.extend({
     },
 });
 
+const routeWillChangeStub = sinon.stub();
+
 function routingAssertions(assert: Assert, segment: string, url: string, route: string) {
     assert.equal(currentURL(), `/${segment}${url}`, `The "real" URL contains the hidden "${segment}" segment`);
     assert.equal(currentLocationURL(), url, 'The Location URL is the same');
     assert.equal(currentRouteName(), route, 'The correct route was reached');
 
-    // eslint-disable-next-line ember/no-private-routing-service
-    const router = (getContext() as TestContext).owner.lookup('router:main');
-    const flagStub: SinonStub = router._beforeTransition;
     assert.ok(
-        flagStub.args.any(
+        routeWillChangeStub.args.any(
             ([t]: [Transition]) => t.targetName === route,
         ),
         `The router checked for a feature flag for "${route}"`,
@@ -44,9 +42,8 @@ module('Acceptance | resolve-guid', hooks => {
     setupMirage(hooks);
 
     hooks.beforeEach(function(this: TestContext) {
-        // eslint-disable-next-line ember/no-private-routing-service
-        const router = this.owner.lookup('router:main');
-        sinon.stub(router, '_beforeTransition').returnsArg(0);
+        const router = this.owner.lookup('service:router');
+        router.on('routeWillChange', routeWillChangeStub);
     });
 
     test('File | Index', async assert => {

tests/engines/registries/integration/components/page-link/component-test.ts

@@ -31,6 +31,7 @@ module('Registries | Integration | Component | page-link', hooks => {
     setupEngineRenderingTest(hooks, 'registries');
 
     hooks.beforeEach(function(this: TestContext) {
+        this.owner.unregister('service:router');
         this.owner.register('service:router', RouterStub);
         this.owner.register('service:osf-router', RouterStub);
         this.owner.register('service:current-user', CurrentUserStub);

tests/engines/registries/integration/components/side-nav/component-test.ts

@@ -31,6 +31,7 @@ module('Registries | Integration | Component | side-nav', hooks => {
     setupEngineRenderingTest(hooks, 'registries');
 
     hooks.beforeEach(function(this: TestContext) {
+        this.owner.unregister('service:router');
         this.owner.register('service:router', RouterStub);
         this.owner.register('service:osf-router', RouterStub);
         this.owner.register('service:current-user', CurrentUserStub);

tests/integration/components/ancestry-display/component-test.ts

@@ -22,6 +22,7 @@ module('Integration | Component | ancestry-display', hooks => {
 
     hooks.beforeEach(function(this: TestContext) {
         this.store = this.owner.lookup('service:store');
+        this.owner.unregister('service:router');
         this.owner.register('service:router', OsfLinkRouterStub.extend({
             urlFor(__: any, modelId: string) {
                 return `/${modelId}`;

tests/integration/components/contributors/component-test.ts

@@ -25,6 +25,7 @@ module('Integration | Component | contributors', hooks => {
     hooks.beforeEach(function(this: ThisTestContext) {
         this.store = this.owner.lookup('service:store');
         this.currentUser = this.owner.lookup('service:current-user');
+        this.owner.unregister('service:router');
         this.owner.register('service:router', OsfLinkRouterStub);
         server.loadFixtures('schema-blocks');
         server.loadFixtures('registration-schemas');

tests/integration/components/draft-registration-card/component-test.ts

@@ -14,6 +14,7 @@ module('Integration | Component | draft-registration-card', hooks => {
     hooks.beforeEach(function(this: TestContext) {
         this.store = this.owner.lookup('service:store');
         this.intl = this.owner.lookup('service:intl');
+        this.owner.unregister('service:router');
         this.owner.register('service:router', OsfLinkRouterStub);
         server.loadFixtures('schema-blocks');
         server.loadFixtures('registration-schemas');