[CLEANUP] remove branching for Ember version < 4

Turns out that accounts for _all_ of the current usage.
emberjs · Nov 22, 2024 · fd10660 · fd10660
1 parent 51ae99b
commit fd10660
Show file tree

Hide file tree

Showing 27 changed files with 235 additions and 602 deletions.
diff --git a/addon/src/setup-application-context.ts b/addon/src/setup-application-context.ts
@@ -4,21 +4,17 @@ import {
   isTestContext,
   type TestContext,
 } from './setup-context.ts';
-import hasEmberVersion from './has-ember-version.ts';
 import settled from './settled.ts';
 import getTestMetadata from './test-metadata.ts';
 import { runHooks } from './helper-hooks.ts';
-import type Router from '@ember/routing/router';
 import type RouterService from '@ember/routing/router-service';
 import { assert } from '@ember/debug';
 
 export interface ApplicationTestContext extends TestContext {
   element?: Element | null;
 }
 
-const CAN_USE_ROUTER_EVENTS = hasEmberVersion(3, 6);
 let routerTransitionsPending: boolean | null = null;
-const ROUTER = new WeakMap();
 const HAS_SETUP_ROUTER = new WeakMap();
 
 // eslint-disable-next-line require-jsdoc
@@ -35,33 +31,7 @@ export function isApplicationTestContext(
   @returns {(boolean|null)} if there are pending transitions
 */
 export function hasPendingTransitions(): boolean | null {
-  if (CAN_USE_ROUTER_EVENTS) {
-    return routerTransitionsPending;
-  }
-
-  const context = getContext();
-
-  // there is no current context, we cannot check
-  if (context === undefined) {
-    return null;
-  }
-
-  const router = ROUTER.get(context);
-
-  if (router === undefined) {
-    // if there is no router (e.g. no `visit` calls made yet), we cannot
-    // check for pending transitions but this is explicitly not an error
-    // condition
-    return null;
-  }
-
-  const routerMicrolib = router._routerMicrolib || router.router;
-
-  if (routerMicrolib === undefined) {
-    return null;
-  }
-
-  return !!routerMicrolib.activeTransition;
+  return routerTransitionsPending;
 }
 
 /**
@@ -87,29 +57,19 @@ export function setupRouterSettlednessTracking() {
   HAS_SETUP_ROUTER.set(context, true);
 
   const { owner } = context;
-  let router: Router | RouterService;
-  if (CAN_USE_ROUTER_EVENTS) {
-    // SAFETY: unfortunately we cannot `assert` here at present because the
-    // class is not exported, only the type, since it is not designed to be
-    // sub-classed. The most we can do at present is assert that it at least
-    // *exists* and assume that if it does exist, it is bound correctly.
-    const routerService = owner.lookup('service:router');
-    assert('router service is not set up correctly', !!routerService);
-    router = routerService as RouterService;
-
-    // track pending transitions via the public routeWillChange / routeDidChange APIs
-    // routeWillChange can fire many times and is only useful to know when we have _started_
-    // transitioning, we can then use routeDidChange to signal that the transition has settled
-    router.on('routeWillChange', () => (routerTransitionsPending = true));
-    router.on('routeDidChange', () => (routerTransitionsPending = false));
-  } else {
-    // SAFETY: similarly, this cast cannot be made safer because on the versions
-    // where we fall into this path, this is *also* not an exported class.
-    const mainRouter = owner.lookup('router:main');
-    assert('router:main is not available', !!mainRouter);
-    router = mainRouter as Router;
-    ROUTER.set(context, router);
-  }
+
+  // SAFETY: unfortunately we cannot `assert` here at present because the
+  // class is not exported, only the type, since it is not designed to be
+  // sub-classed. The most we can do at present is assert that it at least
+  // *exists* and assume that if it does exist, it is bound correctly.
+  const router = owner.lookup('service:router') as RouterService;
+  assert('router service is not set up correctly', !!router);
+
+  // track pending transitions via the public routeWillChange / routeDidChange APIs
+  // routeWillChange can fire many times and is only useful to know when we have _started_
+  // transitioning, we can then use routeDidChange to signal that the transition has settled
+  router.on('routeWillChange', () => (routerTransitionsPending = true));
+  router.on('routeDidChange', () => (routerTransitionsPending = false));
 
   // hook into teardown to reset local settledness state
   const ORIGINAL_WILL_DESTROY = router.willDestroy;
@@ -196,8 +156,6 @@ export function currentRouteName(): string {
   return currentRouteName;
 }
 
-const HAS_CURRENT_URL_ON_ROUTER = hasEmberVersion(2, 13);
-
 /**
   @public
   @returns {string} the applications current url
@@ -211,30 +169,22 @@ export function currentURL(): string {
   }
 
   const router = context.owner.lookup('router:main') as any;
+  const routerCurrentURL = router.currentURL;
+
+  // SAFETY: this path is a lie for the sake of the public-facing types. The
+  // framework itself sees this path, but users never do. A user who calls the
+  // API without calling `visit()` will see an `UnrecognizedURLError`, rather
+  // than getting back `null`.
+  if (routerCurrentURL === null) {
+    return routerCurrentURL as never as string;
+  }
 
-  if (HAS_CURRENT_URL_ON_ROUTER) {
-    const routerCurrentURL = router.currentURL;
-
-    // SAFETY: this path is a lie for the sake of the public-facing types. The
-    // framework itself sees this path, but users never do. A user who calls the
-    // API without calling `visit()` will see an `UnrecognizedURLError`, rather
-    // than getting back `null`.
-    if (routerCurrentURL === null) {
-      return routerCurrentURL as never as string;
-    }
-
-    assert(
-      `currentUrl should be a string, but was ${typeof routerCurrentURL}`,
-      typeof routerCurrentURL === 'string',
-    );
+  assert(
+    `currentUrl should be a string, but was ${typeof routerCurrentURL}`,
+    typeof routerCurrentURL === 'string',
+  );
 
-    return routerCurrentURL;
-  } else {
-    // SAFETY: this is *positively ancient* and should probably be removed at
-    // some point; old routers which don't have `currentURL` *should* have a
-    // `location` with `getURL()` per the docs for 2.12.
-    return (router.location as any).getURL();
-  }
+  return routerCurrentURL;
 }
 
 /**

diff --git a/addon/src/setup-rendering-context.ts b/addon/src/setup-rendering-context.ts
@@ -12,7 +12,6 @@ import type { Owner } from './build-owner.ts';
 import getTestMetadata from './test-metadata.ts';
 import { assert } from '@ember/debug';
 import { runHooks } from './helper-hooks.ts';
-import hasEmberVersion from './has-ember-version.ts';
 import isComponent from './-internal/is-component.ts';
 import { ComponentRenderMap, SetUsage } from './setup-context.ts';
 
@@ -187,20 +186,6 @@ export function render(
       };
       toplevelView.setOutletState(outletState);
 
-      // Ember's rendering engine is integration with the run loop so that when a run
-      // loop starts, the rendering is scheduled to be done.
-      //
-      // Ember should be ensuring an instance on its own here (the act of
-      // setting outletState should ensureInstance, since we know we need to
-      // render), but on Ember < 3.23 that is not guaranteed.
-      if (!hasEmberVersion(3, 23)) {
-        // SAFETY: this was correct and type checked on the Ember v3 types, but
-        // since the `run` namespace does not exist in Ember v4, this no longer
-        // can be type checked. When (eventually) dropping support for Ember v3,
-        // and therefore for versions before 3.23, this can be removed entirely.
-        (run as any).backburner.ensureInstance();
-      }
-
       // returning settled here because the actual rendering does not happen until
       // the renderer detects it is dirty (which happens on backburner's end
       // hook), see the following implementation details:

diff --git a/test-app/tests/integration/rerender-test.js b/test-app/tests/integration/rerender-test.js
@@ -18,13 +18,8 @@ import GlimmerComponent from '@glimmer/component';
 import { tracked } from '@glimmer/tracking';
 import { hbs } from 'ember-cli-htmlbars';
 import { module, test } from 'qunit';
-import hasEmberVersion from '@ember/test-helpers/has-ember-version';
 
 module('rerender real-world', function (hooks) {
-  // this test requires Ember 3.25 in order to use lexically scoped values in templates
-  if (!hasEmberVersion(3, 25)) {
-    return;
-  }
   hooks.beforeEach(async function () {
     await setupContext(this);
     await setupRenderingContext(this);

diff --git a/test-app/tests/integration/settled-test.js b/test-app/tests/integration/settled-test.js
@@ -12,7 +12,6 @@ import {
   render,
   rerender,
 } from '@ember/test-helpers';
-import hasEmberVersion from '@ember/test-helpers/has-ember-version';
 import { module, test } from 'qunit';
 import { hbs } from 'ember-cli-htmlbars';
 import Pretender from 'pretender';
@@ -119,10 +118,6 @@ const TestComponent5 = Component.extend({
 });
 
 module('settled real-world scenarios', function (hooks) {
-  if (!hasEmberVersion(2, 4)) {
-    return;
-  }
-
   hooks.beforeEach(async function () {
     await setupContext(this);
     await setupRenderingContext(this);

diff --git a/test-app/tests/integration/setup-rendering-context-test.js b/test-app/tests/integration/setup-rendering-context-test.js
@@ -15,7 +15,6 @@ import {
 } from '@ember/test-helpers';
 import templateOnly from '@ember/component/template-only';
 
-import hasEmberVersion from '@ember/test-helpers/has-ember-version';
 import { setResolverRegistry } from '../helpers/resolver';
 import { hbs } from 'ember-cli-htmlbars';
 import { precompileTemplate } from '@ember/template-compilation';
@@ -149,60 +148,56 @@ module('setupRenderingContext "real world"', function (hooks) {
   });
 
   module('lexical scope access', function () {
-    if (hasEmberVersion(3, 28)) {
-      test('can render components passed as locals', async function (assert) {
-        let add = helper(function ([first, second]) {
-          return first + second;
-        });
-
-        await render(
-          precompileTemplate('{{add 1 3}}', {
-            scope() {
-              return { add };
-            },
-          })
-        );
-
-        assert.equal(this.element.textContent, '4');
+    test('can render components passed as locals', async function (assert) {
+      let add = helper(function ([first, second]) {
+        return first + second;
       });
-    }
+
+      await render(
+        precompileTemplate('{{add 1 3}}', {
+          scope() {
+            return { add };
+          },
+        })
+      );
+
+      assert.equal(this.element.textContent, '4');
+    });
   });
 
   module('render with a component', function () {
-    if (hasEmberVersion(3, 25)) {
-      test('can render locally defined components', async function (assert) {
-        class MyComponent extends GlimmerComponent {}
+    test('can render locally defined components', async function (assert) {
+      class MyComponent extends GlimmerComponent {}
 
-        setComponentTemplate(hbs`my name is {{@name}}`, MyComponent);
+      setComponentTemplate(hbs`my name is {{@name}}`, MyComponent);
 
-        const somePerson = new (class {
-          @tracked name = 'Zoey';
-        })();
+      const somePerson = new (class {
+        @tracked name = 'Zoey';
+      })();
 
-        const template = precompileTemplate(
-          '<MyComponent @name={{somePerson.name}} />',
-          {
-            scope() {
-              return {
-                somePerson,
-                MyComponent,
-              };
-            },
-          }
-        );
+      const template = precompileTemplate(
+        '<MyComponent @name={{somePerson.name}} />',
+        {
+          scope() {
+            return {
+              somePerson,
+              MyComponent,
+            };
+          },
+        }
+      );
 
-        const component = setComponentTemplate(template, templateOnly());
+      const component = setComponentTemplate(template, templateOnly());
 
-        await render(component);
+      await render(component);
 
-        assert.equal(this.element.textContent, 'my name is Zoey');
+      assert.equal(this.element.textContent, 'my name is Zoey');
 
-        somePerson.name = 'Tomster';
+      somePerson.name = 'Tomster';
 
-        await rerender();
+      await rerender();
 
-        assert.equal(this.element.textContent, 'my name is Tomster');
-      });
-    }
+      assert.equal(this.element.textContent, 'my name is Tomster');
+    });
   });
 });
diff --git a/test-app/tests/unit/dom/blur-test.js b/test-app/tests/unit/dom/blur-test.js
@@ -8,7 +8,6 @@ import {
 } from '@ember/test-helpers';
 import { buildInstrumentedElement, insertElement } from '../../helpers/events';
 import { isEdge, isChrome } from '../../helpers/browser-detect';
-import hasEmberVersion from '@ember/test-helpers/has-ember-version';
 import { createDescriptor } from 'dom-element-descriptors';
 
 let focusSteps = ['focus', 'focusin'];
@@ -23,10 +22,6 @@ if (isChrome) {
 }
 
 module('DOM Helper: blur', function (hooks) {
-  if (!hasEmberVersion(2, 4)) {
-    return;
-  }
-
   let context, elementWithFocus;
 
   hooks.beforeEach(async function (assert) {

diff --git a/test-app/tests/unit/dom/click-test.js b/test-app/tests/unit/dom/click-test.js
@@ -5,7 +5,6 @@ import {
   instrumentElement,
   insertElement,
 } from '../../helpers/events';
-import hasEmberVersion from '@ember/test-helpers/has-ember-version';
 import { isChrome } from '../../helpers/browser-detect';
 import {
   registerHooks,
@@ -15,10 +14,6 @@ import {
 import { createDescriptor } from 'dom-element-descriptors';
 
 module('DOM Helper: click', function (hooks) {
-  if (!hasEmberVersion(2, 4)) {
-    return;
-  }
-
   let context, element;
 
   hooks.beforeEach(function () {

diff --git a/test-app/tests/unit/dom/double-click-test.js b/test-app/tests/unit/dom/double-click-test.js
@@ -10,7 +10,6 @@ import {
   instrumentElement,
   insertElement,
 } from '../../helpers/events';
-import hasEmberVersion from '@ember/test-helpers/has-ember-version';
 import {
   registerHooks,
   unregisterHooks,
@@ -33,10 +32,6 @@ if (isChrome) {
 }
 
 module('DOM Helper: doubleClick', function (hooks) {
-  if (!hasEmberVersion(2, 4)) {
-    return;
-  }
-
   let context, element;
 
   hooks.beforeEach(function () {

diff --git a/test-app/tests/unit/dom/fill-in-test.js b/test-app/tests/unit/dom/fill-in-test.js
@@ -2,7 +2,6 @@ import { module, test } from 'qunit';
 import { fillIn, setupContext, teardownContext } from '@ember/test-helpers';
 import { buildInstrumentedElement, insertElement } from '../../helpers/events';
 import { isFirefox, isChrome } from '../../helpers/browser-detect';
-import hasEmberVersion from '@ember/test-helpers/has-ember-version';
 import {
   registerHooks,
   unregisterHooks,
@@ -17,10 +16,6 @@ if (isFirefox || isChrome) {
 }
 
 module('DOM Helper: fillIn', function (hooks) {
-  if (!hasEmberVersion(2, 4)) {
-    return;
-  }
-
   let context, element;
 
   hooks.beforeEach(function () {