Merge pull request #1504 from kaliber5/deprecate-pos-args

Deprecate positional arguments for link-to components

addon/components/bs-link-to.js

@@ -2,7 +2,7 @@
 import Component from '@ember/component';
 import { tagName } from '@ember-decorators/component';
 import { inject as service } from '@ember/service';
-import { assert } from '@ember/debug';
+import { assert, deprecate } from '@ember/debug';
 import ComponentChild from '../mixins/component-child';
 import { dependentKeyCompat } from '@ember/object/compat';
 
@@ -70,6 +70,17 @@ class LinkComponent extends Component.extend(ComponentChild) {
       return;
     }
 
+    deprecate(
+      `Positional arguments for ember-bootstrap's link-to components are deprecated. Switch to angle bracket invocation and named arguments.`,
+      false,
+      {
+        id: `ember-bootstrap.link-to.positional-args`,
+        until: '5.0.0',
+        since: '4.7.0',
+        for: 'ember-bootstrap',
+      }
+    );
+
     params = params.slice();
 
     // taken from original Ember.LnkComponent

tests/helpers/setup-no-deprecations.js

@@ -2,7 +2,8 @@ import QUnit from 'qunit';
 import { registerDeprecationHandler } from '@ember/debug';
 
 let isRegistered = false;
-let deprecations;
+let deprecations = new Set();
+let expectedDeprecations = new Set();
 
 // Ignore deprecations that are not caused by our own code, and which we cannot fix easily.
 const ignoredDeprecations = [
@@ -13,11 +14,12 @@ const ignoredDeprecations = [
 
 export default function setupNoDeprecations({ beforeEach, afterEach }) {
   beforeEach(function () {
-    deprecations = [];
+    deprecations.clear();
+    expectedDeprecations.clear();
     if (!isRegistered) {
       registerDeprecationHandler((message, options, next) => {
         if (!ignoredDeprecations.some((regex) => message.match(regex))) {
-          deprecations.push(message);
+          deprecations.add(message);
         }
         next(message, options);
       });
@@ -27,31 +29,38 @@ export default function setupNoDeprecations({ beforeEach, afterEach }) {
 
   afterEach(function (assert) {
     // guard in if instead of using assert.equal(), to not make assert.expect() fail
-    if (deprecations && deprecations.length > 0) {
-      assert.ok(false, `Expected no deprecations, found: ${deprecations.map((msg) => `"${msg}"`).join(', ')}`);
+    if (deprecations.size > expectedDeprecations.size) {
+      assert.ok(
+        false,
+        `Expected ${expectedDeprecations.size} deprecations, found: ${[...deprecations]
+          .map((msg) => `"${msg}"`)
+          .join(', ')}`
+      );
     }
   });
 
   QUnit.assert.deprecations = function (count) {
     if (count === undefined) {
-      this.ok(deprecations.length, 'Expected deprecations during test.');
+      this.ok(deprecations.size, 'Expected deprecations during test.');
     } else {
-      this.equal(deprecations.length, count, `Expected ${count} deprecation(s) during test.`);
+      this.equal(deprecations.size, count, `Expected ${count} deprecation(s) during test.`);
     }
 
-    deprecations = [];
+    deprecations.forEach((d) => expectedDeprecations.add(d));
   };
 
   QUnit.assert.deprecationsInclude = function (expected) {
-    let found = deprecations.find((deprecation) => deprecation.includes(expected));
+    let found = [...deprecations].find((deprecation) => deprecation.includes(expected));
     this.pushResult({
       result: !!found,
       actual: deprecations,
-      message: `expected to find \`${expected}\` deprecation. Found ${deprecations.map((d) => `!${d}`).join(', ')}`,
+      message: `expected to find \`${expected}\` deprecation. Found ${[...deprecations]
+        .map((d) => `"${d}"`)
+        .join(', ')}`,
     });
 
     if (found) {
-      deprecations.splice(deprecations.indexOf(found), 1);
+      expectedDeprecations.add(found);
     }
   };
 }

tests/integration/components/bs-dropdown/menu/link-to-test.js

@@ -7,6 +7,8 @@ import setupNoDeprecations from '../../../../helpers/setup-no-deprecations';
 
 module('Integration | Component | bs-dropdown/menu/link-to', function (hooks) {
   setupRenderingTest(hooks);
+  setupNoDeprecations(hooks);
+
   hooks.beforeEach(function () {
     this.owner.setupRouter();
   });
@@ -39,7 +41,11 @@ module('Integration | Component | bs-dropdown/menu/link-to', function (hooks) {
     assert.dom('a.dropdown-item').exists({ count: 1 }, 'renders as plain element with dropdown item class');
   });
 
-  module('positional params', function () {
+  module('positional params', function (hooks) {
+    hooks.afterEach(function (assert) {
+      assert.deprecationsInclude(`Positional arguments for ember-bootstrap's link-to components are deprecated.`);
+      assert.deprecations();
+    });
     test('simple route link', async function (assert) {
       await render(hbs`
         <BsDropdown as |dd|>
@@ -83,9 +89,7 @@ module('Integration | Component | bs-dropdown/menu/link-to', function (hooks) {
     });
   });
 
-  module('<LinkTo> properties', function (hooks) {
-    setupNoDeprecations(hooks);
-
+  module('<LinkTo> properties', function () {
     test('simple route link', async function (assert) {
       await render(hbs`
         <BsDropdown as |dd|>

tests/integration/components/bs-nav/link-to-test.js

@@ -7,11 +7,16 @@ import { testNotBS3 } from '../../../helpers/bootstrap';
 
 module('Integration | Component | bs-nav/link-to', function (hooks) {
   setupRenderingTest(hooks);
+  setupNoDeprecations(hooks);
   hooks.beforeEach(function () {
     this.owner.setupRouter();
   });
 
-  module('positional params', function () {
+  module('positional params', function (hooks) {
+    hooks.afterEach(function (assert) {
+      assert.deprecationsInclude(`Positional arguments for ember-bootstrap's link-to components are deprecated.`);
+      assert.deprecations();
+    });
     test('simple route link', async function (assert) {
       await render(hbs`
         <BsNav as |nav|>
@@ -62,9 +67,7 @@ module('Integration | Component | bs-nav/link-to', function (hooks) {
     });
   });
 
-  module('<LinkTo> properties', function (hooks) {
-    setupNoDeprecations(hooks);
-
+  module('<LinkTo> properties', function () {
     test('simple route link', async function (assert) {
       await render(hbs`
         <BsNav as |nav|>