Merge pull request #20669 from emberjs/kg-break-deprecations-at-until-2

[FEATURE] Make deprecations throw when the `until` for `ember-source` has passed

.github/workflows/ci.yml

@@ -102,6 +102,29 @@ jobs:
           ALL_DEPRECATIONS_ENABLED: true
         run: pnpm test
 
+  deprecations-broken-test:
+    name: Debug and Prebuilt (All Tests by Package + Canary Features) with Deprecations as Errors
+    runs-on: ubuntu-latest
+    needs: [ basic-test, lint, types ]
+    steps:
+      - uses: actions/checkout@v3
+      - uses: ./.github/actions/setup
+      - name: build
+        env:
+          DISABLE_SOURCE_MAPS: true
+          BROCCOLI_ENV: production
+        run: pnpm ember build
+      - name: Upload build
+        uses: actions/upload-artifact@v3
+        with:
+          name: dist
+          path: dist
+      - name: test
+        env:
+          TEST_SUITE: each-package
+          OVERRIDE_DEPRECATION_VERSION: '15.0.0' # Throws on all deprecations with an until before or equal to this version
+        run: pnpm test
+
   browserstack-test:
     name: Browserstack Tests (Safari, Edge)
     runs-on: ubuntu-latest

CONTRIBUTING.md

@@ -309,13 +309,13 @@ can be constructed in advance of the deprecation being added to the
 [deprecation app](https://github.com/ember-learn/deprecation-app) by following 
 this format: `https://deprecations.emberjs.com/deprecations/{{id}}`.
 
-`deprecate` should then be called using the entry from the `DEPRECATIONS` object.
+`deprecateUntil` (internal to Ember) should then be called using the entry from the `DEPRECATIONS` object.
 
 ```ts
-import { DEPRECATIONS } from '@ember/-internals/deprecations';
+import { DEPRECATIONS, deprecateUntil } from '@ember/-internals/deprecations';
 //...
 
-deprecate(message, DEPRECATIONS.MY_DEPRECATION.test, DEPRECATIONS.MY_DEPRECATION.options);
+deprecateUntil(message, DEPRECATIONS.MY_DEPRECATION);
 
 ```
 

bin/run-tests.js

@@ -43,6 +43,10 @@ function run(queryString) {
     queryString = `${queryString}&alldeprecationsenabled=${process.env.ALL_DEPRECATIONS_ENABLED}`;
   }
 
+  if (process.env.OVERRIDE_DEPRECATION_VERSION) {
+    queryString = `${queryString}&overridedeprecationversion=${process.env.OVERRIDE_DEPRECATION_VERSION}`;
+  }
+
   let url = 'http://localhost:' + PORT + '/tests/?' + queryString;
   return runInBrowser(url, 3);
 }

packages/@ember/-internals/deprecations/index.ts

@@ -1,15 +1,37 @@
 import type { DeprecationOptions } from '@ember/debug/lib/deprecate';
 import { ENV } from '@ember/-internals/environment';
+import { VERSION } from '@ember/version';
+import { deprecate, assert } from '@ember/debug';
 
 function isEnabled(options: DeprecationOptions) {
   return Object.hasOwnProperty.call(options.since, 'enabled') || ENV._ALL_DEPRECATIONS_ENABLED;
 }
 
+let numEmberVersion = parseFloat(ENV._OVERRIDE_DEPRECATION_VERSION ?? VERSION);
+
+/* until must only be a minor version or major version */
+export function emberVersionGte(until: string, emberVersion = numEmberVersion) {
+  let significantUntil = until.replace(/(\.0+)/g, '');
+  return emberVersion >= parseFloat(significantUntil);
+}
+
+export function isRemoved(options: DeprecationOptions) {
+  return emberVersionGte(options.until);
+}
+
+interface DeprecationObject {
+  options: DeprecationOptions;
+  test: boolean;
+  isEnabled: boolean;
+  isRemoved: boolean;
+}
+
 function deprecation(options: DeprecationOptions) {
   return {
     options,
     test: !isEnabled(options),
     isEnabled: isEnabled(options),
+    isRemoved: isRemoved(options),
   };
 }
 
@@ -39,7 +61,7 @@ function deprecation(options: DeprecationOptions) {
   import { DEPRECATIONS } from '@ember/-internals/deprecations';
   //...
 
-  deprecate(message, DEPRECATIONS.MY_DEPRECATION.test, DEPRECATIONS.MY_DEPRECATION.options);
+  deprecateUntil(message, DEPRECATIONS.MY_DEPRECATION);
   ```
 
   `expectDeprecation` should also use the DEPRECATIONS object, but it should be noted
@@ -55,6 +77,17 @@ function deprecation(options: DeprecationOptions) {
     DEPRECATIONS.MY_DEPRECATION.isEnabled
   );
   ```
+
+  Tests can be conditionally run based on whether a deprecation is enabled or not:
+
+  ```ts
+    [`${testUnless(DEPRECATIONS.MY_DEPRECATION.isRemoved)} specific deprecated feature tested only in this test`]
+  ```
+
+  This test will be skipped when the MY_DEPRECATION is removed.
+  When adding a deprecation, we need to guard all the code that will eventually be removed, including tests.
+  For tests that are not specifically testing the deprecated feature, we need to figure out how to
+  test the behavior without encountering the deprecated feature, just as users would.
  */
 export const DEPRECATIONS = {
   DEPRECATE_IMPLICIT_ROUTE_MODEL: deprecation({
@@ -65,3 +98,17 @@ export const DEPRECATIONS = {
     url: 'https://deprecations.emberjs.com/v5.x/#toc_deprecate-implicit-route-model',
   }),
 };
+
+export function deprecateUntil(message: string, deprecation: DeprecationObject) {
+  const { options } = deprecation;
+  assert(
+    'deprecateUntil must only be called for ember-source',
+    Boolean(options.for === 'ember-source')
+  );
+  if (deprecation.isRemoved) {
+    throw new Error(
+      `The API deprecated by ${options.id} was removed in ember-source ${options.until}. The message was: ${message}. Please see ${options.url} for more details.`
+    );
+  }
+  deprecate(message, deprecation.test, options);
+}

packages/@ember/-internals/deprecations/tests/index-test.js

@@ -0,0 +1,137 @@
+import { AbstractTestCase, moduleFor } from 'internal-test-helpers';
+import { deprecateUntil, isRemoved, emberVersionGte } from '../index';
+import { ENV } from '@ember/-internals/environment';
+
+let originalEnvValue;
+
+moduleFor(
+  '@ember/-internals/deprecations',
+  class extends AbstractTestCase {
+    constructor() {
+      super();
+      originalEnvValue = ENV.RAISE_ON_DEPRECATION;
+      ENV.RAISE_ON_DEPRECATION = false;
+    }
+
+    teardown() {
+      ENV.RAISE_ON_DEPRECATION = originalEnvValue;
+    }
+
+    ['@test deprecateUntil throws when deprecation has been removed'](assert) {
+      assert.expect(1);
+
+      let MY_DEPRECATION = {
+        options: {
+          id: 'test',
+          until: '3.0.0',
+          for: 'ember-source',
+          url: 'http://example.com/deprecations/test',
+          since: {
+            available: '1.0.0',
+            enabled: '1.0.0',
+          },
+        },
+        isRemoved: true,
+      };
+
+      assert.throws(
+        () => deprecateUntil('Old long gone api is deprecated', MY_DEPRECATION),
+        /Error: The API deprecated by test was removed in ember-source 3.0.0. The message was: Old long gone api is deprecated. Please see http:\/\/example.com\/deprecations\/test for more details./,
+        'deprecateUntil throws when isRemoved is true on deprecation'
+      );
+    }
+
+    ['@test deprecateUntil does not throw when isRemoved is false on deprecation'](assert) {
+      assert.expect(1);
+
+      let MY_DEPRECATION = {
+        options: {
+          id: 'test',
+          until: '3.0.0',
+          for: 'ember-source',
+          url: 'http://example.com/deprecations/test',
+          since: {
+            available: '1.0.0',
+            enabled: '1.0.0',
+          },
+        },
+        isRemoved: false,
+      };
+
+      deprecateUntil('Deprecation is thrown', MY_DEPRECATION);
+
+      assert.ok(true, 'exception on until was not thrown');
+    }
+    ['@test isRemoved is true when until has passed current ember version'](assert) {
+      assert.expect(1);
+
+      let options = {
+        id: 'test',
+        until: '3.0.0',
+        for: 'ember-source',
+        url: 'http://example.com/deprecations/test',
+        since: { available: '1.0.0', enabled: '1.0.0' },
+      };
+
+      assert.strictEqual(isRemoved(options), true, 'isRemoved is true when until has passed');
+    }
+
+    ['@test isRemoved is false before until has passed current ember version'](assert) {
+      assert.expect(1);
+
+      let options = {
+        id: 'test',
+        until: '30.0.0',
+        for: 'ember-source',
+        url: 'http://example.com/deprecations/test',
+        since: { available: '1.0.0', enabled: '1.0.0' },
+      };
+
+      assert.strictEqual(
+        isRemoved(options),
+        false,
+        'isRemoved is false until the until has passed'
+      );
+    }
+
+    ['@test emberVersionGte returns whether the ember version is greater than or equal to the provided version'](
+      assert
+    ) {
+      assert.strictEqual(
+        emberVersionGte('3.0.0', parseFloat('5.0.0')),
+        true,
+        '5.0.0 is after 3.0.0'
+      );
+      assert.strictEqual(
+        emberVersionGte('30.0.0', parseFloat('5.0.0')),
+        false,
+        '5.0.0 is before 30.0.0'
+      );
+      assert.strictEqual(
+        emberVersionGte('5.0.0-beta.1', parseFloat('5.0.0')),
+        true,
+        '5.0.0 is after 5.0.0-beta.1'
+      );
+      assert.strictEqual(
+        emberVersionGte('5.0.1', parseFloat('5.0.0-beta.1')),
+        false,
+        '5.0.0-beta.1 is before 5.0.1'
+      );
+      assert.strictEqual(
+        emberVersionGte('5.0.0-alpha.abcde', parseFloat('5.0.0')),
+        true,
+        '5.0.0 is after 5.0.0-alpha'
+      );
+      assert.strictEqual(
+        emberVersionGte('5.9.0', parseFloat('5.8.9')),
+        false,
+        '5.8.9 is before 5.9.0'
+      );
+      assert.strictEqual(
+        emberVersionGte('5.10.0', parseFloat('5.9.2')),
+        true,
+        '5.10.1 is after 5.9.2'
+      );
+    }
+  }
+);

packages/@ember/-internals/environment/lib/env.ts

@@ -141,6 +141,18 @@ export const ENV = {
    */
   _ALL_DEPRECATIONS_ENABLED: false,
 
+  /**
+   Override the version of ember-source used to determine when deprecations "break".
+   This is used internally by Ember to test with deprecated features "removed".
+   This is never intended to be set by projects.
+   @property _OVERRIDE_DEPRECATION_VERSION
+   @for EmberENV
+   @type string | null
+   @default null
+   @private
+   */
+  _OVERRIDE_DEPRECATION_VERSION: null,
+
   /**
     Whether the app defaults to using async observers.
 
@@ -214,6 +226,8 @@ export const ENV = {
       (ENV as Record<string, unknown>)[flag] = EmberENV[flag] !== false;
     } else if (defaultValue === false) {
       (ENV as Record<string, unknown>)[flag] = EmberENV[flag] === true;
+    } else {
+      (ENV as Record<string, unknown>)[flag] = EmberENV[flag];
     }
   }
 

packages/@ember/routing/route.ts

@@ -18,8 +18,8 @@ import { isProxy, lookupDescriptor } from '@ember/-internals/utils';
 import type { AnyFn } from '@ember/-internals/utility-types';
 import Controller from '@ember/controller';
 import type { ControllerQueryParamType } from '@ember/controller';
-import { assert, deprecate, info, isTesting } from '@ember/debug';
-import { DEPRECATIONS } from '@ember/-internals/deprecations';
+import { assert, info, isTesting } from '@ember/debug';
+import { DEPRECATIONS, deprecateUntil } from '@ember/-internals/deprecations';
 import EngineInstance from '@ember/engine/instance';
 import { dependentKeyCompat } from '@ember/object/compat';
 import { once } from '@ember/runloop';
@@ -1256,11 +1256,10 @@ class Route<Model = unknown> extends EmberObject.extend(ActionHandler, Evented)
     if (ENV._NO_IMPLICIT_ROUTE_MODEL) {
       return;
     }
-    deprecate(
+    deprecateUntil(
       `The implicit model loading behavior for routes is deprecated. ` +
         `Please define an explicit model hook for ${this.fullRouteName}.`,
-      DEPRECATIONS.DEPRECATE_IMPLICIT_ROUTE_MODEL.test,
-      DEPRECATIONS.DEPRECATE_IMPLICIT_ROUTE_MODEL.options
+      DEPRECATIONS.DEPRECATE_IMPLICIT_ROUTE_MODEL
     );
 
     const store = 'store' in this ? this.store : get(this, '_store');