Merge pull request #16218 from rwjblue/fix-get-with-numeric

[BUGFIX beta] Prevent errors when using const `(get arr 1)`.

Author: rwjblue

packages/ember-glimmer/lib/helpers/get.ts

@@ -79,8 +79,12 @@ class GetHelperReference extends CachedReference {
 
   static create(sourceReference: VersionedPathReference<Opaque>, pathReference: PathReference<string>) {
     if (isConst(pathReference)) {
-      let parts = pathReference.value().split('.');
-      return referenceFromParts(sourceReference, parts);
+      let value = pathReference.value();
+      if (typeof value === 'string' && value.indexOf('.') > -1) {
+        return referenceFromParts(sourceReference, value.split('.'));
+      } else {
+        return sourceReference.get(value);
+      }
     } else {
       return new GetHelperReference(sourceReference, pathReference);
     }
@@ -108,10 +112,10 @@ class GetHelperReference extends CachedReference {
       if (path !== undefined && path !== null && path !== '') {
         let pathType = typeof path;
 
-        if (pathType === 'string') {
+        if (pathType === 'string' && path.indexOf('.') > -1) {
           innerReference = referenceFromParts(this.sourceReference, path.split('.'));
-        } else if (pathType === 'number') {
-          innerReference = this.sourceReference.get('' + path);
+        } else {
+          innerReference = this.sourceReference.get(path);
         }
 
         innerTag.inner.update(innerReference.tag);

packages/ember-glimmer/tests/integration/helpers/get-test.js

@@ -50,8 +50,8 @@ moduleFor('Helpers test: {{get}}', class extends RenderingTest {
     this.assertText('[red and yellow] [red and yellow]');
   }
 
-  ['@test should be able to get an object value with numeric keys']() {
-    this.render(`{{#each indexes as |index|}}[{{get items index}}]{{/each}}`, {
+  ['@test should be able to get an object value with a number']() {
+    this.render(`[{{get items 1}}][{{get items 2}}][{{get items 3}}]`, {
       indexes: [1, 2, 3],
       items: {
         1: 'First',
@@ -75,8 +75,52 @@ moduleFor('Helpers test: {{get}}', class extends RenderingTest {
     this.assertText('[First][Second][Third]');
   }
 
+  ['@test should be able to get an array value with a number']() {
+    this.render(`[{{get numbers 0}}][{{get numbers 1}}][{{get numbers 2}}]`, {
+      numbers: [1, 2, 3],
+    });
+
+    this.assertText('[1][2][3]');
+
+    this.runTask(() => this.rerender());
+
+    this.assertText('[1][2][3]');
+
+    this.runTask(() => set(this.context, 'numbers', [3, 2, 1]));
+
+    this.assertText('[3][2][1]');
+
+    this.runTask(() => set(this.context, 'numbers', [1, 2, 3]));
+
+    this.assertText('[1][2][3]');
+  }
+
+  ['@test should be able to get an object value with a path evaluating to a number']() {
+    this.render(`{{#each indexes as |index|}}[{{get items index}}]{{/each}}`, {
+      indexes: [1, 2, 3],
+      items: {
+        1: 'First',
+        2: 'Second',
+        3: 'Third'
+      }
+    });
+
+    this.assertText('[First][Second][Third]');
+
+    this.runTask(() => this.rerender());
+
+    this.assertText('[First][Second][Third]');
+
+    this.runTask(() => set(this.context, 'items.1', 'Qux'));
+
+    this.assertText('[Qux][Second][Third]');
+
+    this.runTask(() => set(this.context, 'items', { 1: 'First', 2: 'Second', 3: 'Third' }));
+
+    this.assertText('[First][Second][Third]');
+  }
 
-  ['@test should be able to get an array value with numeric keys']() {
+  ['@test should be able to get an array value with a path evaluating to a number']() {
     this.render(`{{#each numbers as |num index|}}[{{get numbers index}}]{{/each}}`, {
       numbers: [1, 2, 3],
     });

packages/ember-metal/lib/path_cache.js

@@ -3,5 +3,5 @@ import Cache from './cache';
 const firstDotIndexCache = new Cache(1000, key => key.indexOf('.'));
 
 export function isPath(path) {
-  return firstDotIndexCache.get(path) !== -1;
+  return typeof path === 'string' && firstDotIndexCache.get(path) !== -1;
 }

packages/ember-metal/lib/property_get.js

@@ -72,8 +72,8 @@ export function getPossibleMandatoryProxyValue(obj, keyName) {
 export function get(obj, keyName) {
   assert(`Get must be called with two arguments; an object and a property key`, arguments.length === 2);
   assert(`Cannot call get with '${keyName}' on an undefined object.`, obj !== undefined && obj !== null);
-  assert(`The key provided to get must be a string, you passed ${keyName}`, typeof keyName === 'string');
-  assert(`'this' in paths is not supported`, keyName.lastIndexOf('this.', 0) !== 0);
+  assert(`The key provided to get must be a string or number, you passed ${keyName}`, typeof keyName === 'string' || (typeof keyName === 'number' && !isNaN(keyName)));
+  assert(`'this' in paths is not supported`, typeof keyName !== 'string' || keyName.lastIndexOf('this.', 0) !== 0);
   assert('Cannot call `get` with an empty string', keyName !== '');
 
   let type = typeof obj;

packages/ember-metal/lib/property_set.js

@@ -45,8 +45,8 @@ export function set(obj, keyName, value, tolerant) {
     arguments.length === 3 || arguments.length === 4
   );
   assert(`Cannot call set with '${keyName}' on an undefined object.`, obj && typeof obj === 'object' || typeof obj === 'function');
-  assert(`The key provided to set must be a string, you passed ${keyName}`, typeof keyName === 'string');
-  assert(`'this' in paths is not supported`, keyName.lastIndexOf('this.', 0) !== 0);
+  assert(`The key provided to set must be a string or number, you passed ${keyName}`, typeof keyName === 'string' || (typeof keyName === 'number' && !isNaN(keyName)));
+  assert(`'this' in paths is not supported`, typeof keyName !== 'string' || keyName.lastIndexOf('this.', 0) !== 0);
   assert(`calling set on destroyed object: ${toString(obj)}.${keyName} = ${toString(value)}`, !obj.isDestroyed);
 
   if (isPath(keyName)) {

packages/ember-metal/tests/accessors/get_test.js

@@ -27,6 +27,19 @@ moduleFor('get', class extends AbstractTestCase {
     }
   }
 
+  ['@test should retrieve a number key on an object'](assert) {
+    let obj = { 1: 'first' };
+
+    assert.equal(get(obj, 1), 'first');
+  }
+
+  ['@test should retrieve an array index'](assert) {
+    let arr = ['first', 'second'];
+
+    assert.equal(get(arr, 0), 'first');
+    assert.equal(get(arr, 1), 'second');
+  }
+
   ['@test should not access a property more than once'](assert) {
     let count = 0;
     let obj = {
@@ -106,11 +119,10 @@ moduleFor('get', class extends AbstractTestCase {
 
   ['@test warn on attempts to use get with an unsupported property path']() {
     let obj = {};
-    expectAssertion(() => get(obj, null),      /The key provided to get must be a string, you passed null/);
-    expectAssertion(() => get(obj, NaN),       /The key provided to get must be a string, you passed NaN/);
-    expectAssertion(() => get(obj, undefined), /The key provided to get must be a string, you passed undefined/);
-    expectAssertion(() => get(obj, false),     /The key provided to get must be a string, you passed false/);
-    expectAssertion(() => get(obj, 42),        /The key provided to get must be a string, you passed 42/);
+    expectAssertion(() => get(obj, null),      /The key provided to get must be a string or number, you passed null/);
+    expectAssertion(() => get(obj, NaN),       /The key provided to get must be a string or number, you passed NaN/);
+    expectAssertion(() => get(obj, undefined), /The key provided to get must be a string or number, you passed undefined/);
+    expectAssertion(() => get(obj, false),     /The key provided to get must be a string or number, you passed false/);
     expectAssertion(() => get(obj, ''), /Cannot call `get` with an empty string/);
   }
 

packages/ember-metal/tests/accessors/set_test.js

@@ -34,6 +34,20 @@ moduleFor('set', class extends AbstractTestCase {
     }
   }
 
+  ['@test should set a number key on an object'](assert) {
+    let obj = { };
+
+    set(obj, 1, 'first');
+    assert.equal(obj[1], 'first');
+  }
+
+  ['@test should set an array index'](assert) {
+    let arr = ['first', 'second'];
+
+    set(arr, 1, 'lol');
+    assert.deepEqual(arr, ['first', 'lol']);
+  }
+
   ['@test should call setUnknownProperty if defined and value is undefined'](assert) {
     let obj = {
       count: 0,
@@ -64,11 +78,10 @@ moduleFor('set', class extends AbstractTestCase {
 
   ['@test warn on attempts to use set with an unsupported property path']() {
     let obj = {};
-    expectAssertion(() => set(obj, null, 42),      /The key provided to set must be a string, you passed null/);
-    expectAssertion(() => set(obj, NaN, 42),       /The key provided to set must be a string, you passed NaN/);
-    expectAssertion(() => set(obj, undefined, 42), /The key provided to set must be a string, you passed undefined/);
-    expectAssertion(() => set(obj, false, 42),     /The key provided to set must be a string, you passed false/);
-    expectAssertion(() => set(obj, 42, 42),        /The key provided to set must be a string, you passed 42/);
+    expectAssertion(() => set(obj, null, 42),      /The key provided to set must be a string or number, you passed null/);
+    expectAssertion(() => set(obj, NaN, 42),       /The key provided to set must be a string or number, you passed NaN/);
+    expectAssertion(() => set(obj, undefined, 42), /The key provided to set must be a string or number, you passed undefined/);
+    expectAssertion(() => set(obj, false, 42),     /The key provided to set must be a string or number, you passed false/);
   }
 
   ['@test warn on attempts of calling set on a destroyed object']() {