From 7ebdc05fa9ddf06c3e75e7aba686c03a7ddc5adf Mon Sep 17 00:00:00 2001
From: Chris Garrett <cgarrett@linkedin.com>
Date: Fri, 12 Feb 2021 13:14:59 -0800
Subject: [PATCH] [BUGFIX] Fix {{get}} when used on non-objects

Previously, `{{get}}` would not work on non-object values, even though
they may have had properties (e.g. `String.length`). This PR fixes this
and makes sure that `{{get}}` has the same behavior as directly
referencing the property in a template without using `{{get}}`.
---
 .../integration-tests/lib/modes/env.ts        |  2 +-
 .../test/helpers/get-test.ts                  | 33 +++++++++++++++++++
 packages/@glimmer/runtime/lib/helpers/get.ts  |  9 ++---
 3 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/packages/@glimmer/integration-tests/lib/modes/env.ts b/packages/@glimmer/integration-tests/lib/modes/env.ts
index ddf12d4b47..555751f36d 100644
--- a/packages/@glimmer/integration-tests/lib/modes/env.ts
+++ b/packages/@glimmer/integration-tests/lib/modes/env.ts
@@ -114,7 +114,7 @@ setGlobalContext({
     let current: unknown = obj;
 
     for (let part of parts) {
-      if (typeof current === 'function' || (typeof current === 'object' && current !== null)) {
+      if (current !== null && current !== undefined) {
         current = (current as Record<string, unknown>)[part];
       }
     }
diff --git a/packages/@glimmer/integration-tests/test/helpers/get-test.ts b/packages/@glimmer/integration-tests/test/helpers/get-test.ts
index ac2620c50c..71056fbff6 100644
--- a/packages/@glimmer/integration-tests/test/helpers/get-test.ts
+++ b/packages/@glimmer/integration-tests/test/helpers/get-test.ts
@@ -406,6 +406,39 @@ class GetTest extends RenderTest {
     this.rerender({ age: 30 });
     this.assertHTML('miguelandrade30');
   }
+
+  @test
+  'should be able to get string length with a static key'() {
+    this.render(`[{{get this.name 'length'}}] [{{if true (get this.name 'length')}}]`, {
+      name: 'Tomster',
+    });
+
+    this.assertHTML('[7] [7]');
+    this.assertStableRerender();
+
+    this.rerender({ name: 'Zoey' });
+    this.assertHTML('[4] [4]');
+
+    this.rerender({ name: 'Tomster' });
+    this.assertHTML('[7] [7]');
+  }
+
+  @test
+  'should be able to get string length with a bound/dynamic key'() {
+    this.render(`[{{get this.name this.key}}] [{{if true (get this.name this.key)}}]`, {
+      name: 'Tomster',
+      key: 'length',
+    });
+
+    this.assertHTML('[7] [7]');
+    this.assertStableRerender();
+
+    this.rerender({ key: 'foo' });
+    this.assertHTML('[] []');
+
+    this.rerender({ name: 'Zoey', key: 'length' });
+    this.assertHTML('[4] [4]');
+  }
 }
 
 jitSuite(GetTest);
diff --git a/packages/@glimmer/runtime/lib/helpers/get.ts b/packages/@glimmer/runtime/lib/helpers/get.ts
index c6ca965e37..0f3271d0e8 100644
--- a/packages/@glimmer/runtime/lib/helpers/get.ts
+++ b/packages/@glimmer/runtime/lib/helpers/get.ts
@@ -1,6 +1,7 @@
 import { getPath, setPath } from '@glimmer/global-context';
 import { VMArguments } from '@glimmer/interfaces';
 import { createComputeRef, valueForRef } from '@glimmer/reference';
+import { isDict } from '@glimmer/util';
 import { internalHelper } from './internal-helper';
 
 /**
@@ -88,21 +89,17 @@ export default internalHelper((args: VMArguments) => {
     () => {
       let source = valueForRef(sourceRef);
 
-      if (isObject(source)) {
+      if (isDict(source)) {
         return getPath(source, String(valueForRef(pathRef)));
       }
     },
     (value) => {
       let source = valueForRef(sourceRef);
 
-      if (isObject(source)) {
+      if (isDict(source)) {
         return setPath(source, String(valueForRef(pathRef)), value);
       }
     },
     'get'
   );
 });
-
-function isObject(obj: unknown): obj is object {
-  return typeof obj === 'function' || (typeof obj === 'object' && obj !== null);
-}