Update new collection API signature

Scope parameter is required, so make it the first parameter. This makes
easy to differentiate between old API and new API.

addon/-private/properties/collection.js

@@ -36,8 +36,7 @@ import { collection as legacyCollection } from './collection/legacy';
  * // </table>
  *
  * const page = PageObject.create({
- *   users: collection({
- *     scope: 'table tr',
+ *   users: collection('table tr', {
  *     firstName: text('td', { at: 0 }),
  *     lastName: text('td', { at: 1 })
  *   })
@@ -72,8 +71,7 @@ import { collection as legacyCollection } from './collection/legacy';
  * const page = PageObject.create({
  *   scope: '.admins',
  *
- *   users: collection({
- *     scope: 'table tr',
+ *   users: collection('table tr', {
  *     firstName: text('td', { at: 0 }),
  *     lastName: text('td', { at: 1 })
  *   })
@@ -100,8 +98,7 @@ import { collection as legacyCollection } from './collection/legacy';
  * const page = PageObject.create({
  *   scope: 'table',
  *
- *   users: PageObject.collection({
- *     scope: 'tr',
+ *   users: PageObject.collection('tr', {
  *     firstName: text('td', { at: 0 }),
  *     lastName: text('td', { at: 1 }),
  *   })
@@ -110,44 +107,21 @@ import { collection as legacyCollection } from './collection/legacy';
  * let john = page.users.filter((item) => item.firstName === 'John' )[0];
  * assert.equal(john.lastName, 'Doe');
  *
- * @param {Object} definition - Collection definition
+ * @param {String} scopeOrDefinition - Selector to define the items of the collection
+ * @param {Object} [definitionOrNothing] - Object with the definition of item properties
  * @return {Descriptor}
  */
-export function collection(definition) {
-  if (useLegacyAPI(definition)) {
-    deprecate('You are currently using the legacy collection API, check the documentation to see how to upgrade to the new API.', false, {
-      id: 'ember-cli-page-object.old-collection-api',
-      until: '2.0.0',
-      url: 'https://gist.github.com/san650/17174e4b7b1fd80b049a47eb456a7cdc#file-old-collection-api-js',
-    });
+export function collection(scopeOrDefinition, definitionOrNothing) {
 
-    return legacyCollection(definition);
-  } else {
-    return mainCollection(definition);
+  if (typeof scopeOrDefinition === 'string') {
+    return mainCollection(scopeOrDefinition, definitionOrNothing);
   }
-}
 
-function useLegacyAPI(definition) {
-  // We test the use of `itemScope` attribute to guess that the users are using
-  // the legacy API.
-  //
-  // Also, there's a common mistake in page objects created in the wild that
-  // don't define the `itemScope` attribute but they do implement the `items`
-  // object.
-  //
-  //     var page = create({
-  //       foo: collection({
-  //         scope: '.foo',
-  //         item: {
-  //           text: text('p')
-  //         }
-  //       })
-  //     });
-  //
-  // Although this doesn't make any sense and doesn't work in legacy
-  // collections definitions, using the new collection API in this cases break
-  // tests suites. For that reason we use the legacy definition and print a
-  // deprecation warning.
-  return ('itemScope' in definition) ||
-    ('scope' in definition && 'item' in definition);
+  deprecate('You are currently using the legacy collection API, check the documentation to see how to upgrade to the new API.', false, {
+    id: 'ember-cli-page-object.old-collection-api',
+    until: '2.0.0',
+    url: 'https://gist.github.com/san650/17174e4b7b1fd80b049a47eb456a7cdc#file-old-collection-api-js',
+  });
+
+  return legacyCollection(scopeOrDefinition);
 }

addon/-private/properties/collection/main.js

@@ -10,15 +10,16 @@ import { count } from '../count';
 import Ceibo from 'ceibo';
 
 export class Collection {
-  constructor(definition, parent, key) {
-    this.definition = definition;
+  constructor(scope, definition, parent, key) {
+    this.scope = scope;
+    this.definition = definition || {};
     this.parent = parent;
     this.key = key;
 
     this._itemCounter = create({
-      count: count(definition.scope, {
-        resetScope: definition.resetScope,
-        testContainer: definition.testContainer
+      count: count(scope, {
+        resetScope: this.definition.resetScope,
+        testContainer: this.definition.testContainer
       })
     }, { parent });
 
@@ -33,12 +34,12 @@ export class Collection {
     let { key } = this;
 
     if (typeof this._items[index] === 'undefined') {
-      let { definition, parent } = this;
-      let scope = buildSelector({}, definition.scope, { at: index });
+      let { scope, definition, parent } = this;
+      let itemScope = buildSelector({}, scope, { at: index });
 
       let finalizedDefinition = assign({}, definition);
 
-      finalizedDefinition.scope = scope;
+      finalizedDefinition.scope = itemScope;
 
       let tree = create(finalizedDefinition, { parent });
 
@@ -105,15 +106,15 @@ if (typeof (Symbol) !== 'undefined' && Symbol.iterator) {
   }
 }
 
-export function collection(definition) {
+export function collection(scope, definition) {
   let descriptor = {
     isDescriptor: true,
 
     setup(node, key) {
       // Set the value on the descriptor so that it will be picked up and applied by Ceibo.
       // This does mutate the descriptor, but because `setup` is always called before the
       // value is assigned we are guaranteed to get a new, unique Collection instance each time.
-      descriptor.value = new Collection(definition, node, key);
+      descriptor.value = new Collection(scope, definition, node, key);
     }
   };
 

tests/integration/actions-test.js

@@ -36,18 +36,13 @@ const button = function(scope) {
 };
 
 const page = PageObject.create({
-  numbers: collection({
-    scope: '.numbers',
-    itemScope: 'button',
-
-    item: {
-      click: clickable(),
-      text: text()
-    },
-
-    clickOn: clickOnText()
+  numbers: collection('.numbers button', {
+    click: clickable(),
+    text: text()
   }),
 
+  clickOnNumber: clickOnText('.numbers'),
+
   operators: {
     scope: '.operators',
 
@@ -112,22 +107,22 @@ test('Actions work when defined inside collections', function(assert) {
   this.render(template);
 
   page
-    .numbers(0)
+    .numbers
+    .objectAt(0)
     .click();
 
   assert.equal(page.screen.text, '1');
 });
 
-test('Chaining of actions inside a collection works', function(assert) {
+test('Chaining of custom actions works', function(assert) {
   let template = createCalculatorTemplate();
 
   this.render(template);
 
   page
-    .numbers()
-    .clickOn('1')
-    .clickOn('2')
-    .clickOn('3');
+    .clickOnNumber('1')
+    .clickOnNumber('2')
+    .clickOnNumber('3');
 
   assert.equal(page.screen.text, '123');
 });

tests/integration/deprecations/legacy-collection-test.js

@@ -6,7 +6,7 @@ moduleForComponent('calculating-device', 'Deprecation | legacy collection', {
   integration: true
 });
 
-test('usage of itemScope in definition leads to the deprecation', function(assert) {
+test('shows deprecation warning when first parameter is not a string', function(assert) {
   let page = create({
     foo: collection({
       itemScope: 'li'
@@ -18,25 +18,9 @@ test('usage of itemScope in definition leads to the deprecation', function(asser
   assert.expectDeprecation('You are currently using the legacy collection API, check the documentation to see how to upgrade to the new API.');
 });
 
-test('usage of scope plus item leads to the deprecation', function(assert) {
+test("doesn't show a deprecation warning when first parameter is a string", function(assert) {
   let page = create({
-    foo: collection({
-      scope: 'foo',
-      item: {}
-    })
-  });
-
-  page.foo();
-
-  assert.expectDeprecation('You are currently using the legacy collection API, check the documentation to see how to upgrade to the new API.');
-});
-
-test('usage of scope without item does not lead to the deprecation', function(assert) {
-  let page = create({
-    foo: collection({
-      scope: 'foo',
-      bar: {}
-    })
+    foo: collection('foo')
   });
 
   page.foo;