From 949811f193ae42a4686e519f493925143b490d58 Mon Sep 17 00:00:00 2001
From: Ruslan Hrabovyi <kudgo.test@gmail.com>
Date: Thu, 26 Mar 2020 23:57:09 +0200
Subject: [PATCH] [deprecate] strings in definitions (#485)

in order to unlock any strings to be considered as a scope(#408) in v2.
---
 addon-test-support/create.js                  | 11 ++++-
 guides/deprecations.md                        | 39 +++++++++++++++++-
 .../deprecations/string-properties-test.js    | 41 +++++++++++++++++++
 3 files changed, 89 insertions(+), 2 deletions(-)
 create mode 100644 tests/integration/deprecations/string-properties-test.js

diff --git a/addon-test-support/create.js b/addon-test-support/create.js
index 73f4eb0a..045378b9 100644
--- a/addon-test-support/create.js
+++ b/addon-test-support/create.js
@@ -1,4 +1,5 @@
 import Ceibo from 'ceibo';
+import { deprecate } from '@ember/application/deprecations';
 import { render, setContext, removeContext } from './-private/context';
 import { assign, getPageObjectDefinition, isPageObject, storePageObjectDefinition } from './-private/helpers';
 import { visitable } from './properties/visitable';
@@ -63,7 +64,7 @@ function buildObject(node, blueprintKey, blueprint, defaultBuilder) {
     definition = getPageObjectDefinition(blueprint);
   } else {
     Object.getOwnPropertyNames(blueprint).forEach((key) => {
-      const { get } = Object.getOwnPropertyDescriptor(blueprint, key);
+      const { get, value } = Object.getOwnPropertyDescriptor(blueprint, key);
 
       if (typeof get === 'function') {
         Object.defineProperty(blueprint, key, {
@@ -72,6 +73,14 @@ function buildObject(node, blueprintKey, blueprint, defaultBuilder) {
             get
           }
         });
+      } else {
+        deprecate('do not use string values on definitions',
+          typeof value !== 'string' || ['scope', 'testContainer'].includes(key), {
+            id: 'ember-cli-page-object.string-properties-on-definition',
+            until: "2.0.0",
+            url: 'https://ember-cli-page-object.js.org/docs/v1.17.x/deprecations/#string-properties-on-definition',
+          }
+        )
       }
     });
 
diff --git a/guides/deprecations.md b/guides/deprecations.md
index c6a28faf..1e344db4 100644
--- a/guides/deprecations.md
+++ b/guides/deprecations.md
@@ -5,6 +5,44 @@ title: Deprecations
 
 This is a list of deprecations introduced in 1.x cycle:
 
+## String properties on definition
+
+**ID**: ember-cli-page-object.string-properties-on-definition
+
+**Until**: 2.0.0
+
+In v2, any string values in definitions, other than `scope` and `testContainer`, would be treated as a `scope` selectors.
+
+Please make sure there are no plain strings used in your definitions.
+
+Bad:
+
+```js
+import { create } from 'ember-cli-page-object';
+
+const page = create({
+  scope: 'input',
+
+  propertyName: 'I will become a nested component scope'
+});
+```
+
+If you really need your definition to keep a string, use getter instead:
+
+Good:
+
+```js
+import { create } from 'ember-cli-page-object';
+
+const page = create({
+  scope: 'input',
+
+  get propertyName() {
+    return 'I will not become a scope :(';
+  }
+});
+```
+
 ## Multiple
 
 **ID**: ember-cli-page-object.multiple
@@ -66,7 +104,6 @@ const page = create({
 assert.deepEqual(page.tags, ['one', 'two', 'three'])
 ```
 
-
 ## Is property
 
 **ID**: ember-cli-page-object.is-property
diff --git a/tests/integration/deprecations/string-properties-test.js b/tests/integration/deprecations/string-properties-test.js
new file mode 100644
index 00000000..6ff8e31c
--- /dev/null
+++ b/tests/integration/deprecations/string-properties-test.js
@@ -0,0 +1,41 @@
+import { module, test } from 'qunit';
+import { create } from 'ember-cli-page-object';
+import require from 'require';
+
+if (require.has('@ember/test-helpers')) {
+  module('Deprecation | string-properties', function() {
+    test('works at top-level', function(assert) {
+      this.page = create({
+        stringProp: ''
+      });
+
+      assert.expectDeprecation()
+    });
+
+    test('works for nested definitions', function(assert) {
+      this.page = create({
+        nested: {
+          stringProp: ''
+        }
+      });
+
+      assert.expectDeprecation()
+    });
+
+    test('allows scope', function(assert) {
+      this.page = create({
+        scope: ''
+      });
+
+      assert.expectNoDeprecation()
+    });
+
+    test('allows testContainer', function(assert) {
+      this.page = create({
+        testContainer: ''
+      });
+
+      assert.expectNoDeprecation()
+    });
+  });
+}