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');