Fix for tracked properties updating when using m3 native properties

addon/model.js

@@ -80,6 +80,8 @@ let megamorphicModelProxyHandler, megamorphicNativeDeprecationHandler;
 
 if (CUSTOM_MODEL_CLASS) {
   const MegamorphicModelProxyHandler = class {
+    getting = '';
+
     get(target, key, receiver) {
       if (typeof key !== 'string' || key in target) {
         return Reflect.get(target, key, receiver);
@@ -88,19 +90,45 @@ if (CUSTOM_MODEL_CLASS) {
       // Ideally we would do `receiver.unknownProperty(key)` here, but
       // unfortunately `unknownProperty` does not entangle the property
       // tag. `Ember.get` is the only thing that does, actually, so we
-      // have to use it. This is safe, because we already checked that the
-      // property is not `in` the instance, so it will definitely call
-      // `unknownProperty` and will not re-enter.
-
-      // We have to use `target.get` instead of `receiver.get` because
-      // `Ember.get` will access the property itself before calling
-      // `unknownProperty` causing an infinite recursion
-
-      // The only potential downside here is that the value of `this` in our
-      // `unknownProperty` handler will be set to the target MegamorphicModel
-      //  and not to the proxy, so we have to be careful when accessing WeakMaps
-      // inside our `unknonwProperty` code, but there isn't a user visible impact
-      return target.get(key);
+      // have to use it. However we need to be careful that we do not end
+      // up in an infinite loop when relying on `Ember.get` calling
+      // `.unknownProperty`
+
+      // `Ember.get` is going to check whether `receiver` has the `key`
+      // property defined, and if not, call `.unknownProperty`
+
+      // However, the way that check is implemented differs slightly
+      // between the DEBUG and production ember builds.
+      // See: https://github.com/emberjs/ember.js/blob/3ce13cea235cde8a87d89473533c453523412764/packages/%40ember/-internals/metal/lib/property_get.ts#L110
+
+      // In DEBUG, Ember will have it's own debug proxies installed before this proxy,
+      // and Ember's proxy will have stashed the `target` (in this case the m3 model) under a symbol.
+      // See: https://github.com/emberjs/ember.js/blob/3ce13cea235cde8a87d89473533c453523412764/packages/%40ember/-internals/metal/lib/property_get.ts#L22
+      // To check whether it should call `unknonwnProperty`, `Ember.get` will check for property existance
+      // on the stashed target. Because we have already checked that `key` is not `in target`,
+      // we know the target check will return false, and we are not going re-enter and cause an infinite loop.
+
+      // In production, without Ember's debug Proxies, we will go through
+      // a more straightforward path, where `Ember.get` will do a `receiver[key]` check
+      // to decided whether to call `.unknownProperty`. In that case, we need to make sure
+      // that the second time this getter is called with the key, we return `undefined`
+      // to trigger `unknownProperty`, as otherwise we will end up in an infinite loop
+      // of repeatadly calling `receiver.get`
+
+      if (this.getting === key) {
+        return undefined;
+      }
+
+      let shouldReset = this.getting === '';
+      this.getting = key;
+
+      try {
+        return get(receiver, key);
+      } finally {
+        if (shouldReset) {
+          this.getting = '';
+        }
+      }
     }
 
     set(target, key, value, receiver) {

package.json

@@ -75,6 +75,7 @@
     "@ember-data/store": "3.28.3",
     "@ember/optional-features": "^2.0.0",
     "@ember/test-helpers": "^2.5.0",
+    "@glimmer/component": "^1.0.4",
     "@malleatus/nyx": "^0.2.0",
     "@octokit/rest": "^18.12.0",
     "babel-eslint": "^10.1.0",
@@ -152,7 +153,7 @@
     }
   },
   "volta": {
-    "node": "14.8.0",
+    "node": "14.18.1",
     "yarn": "1.22.10"
   }
 }

tests/unit/model/native-access-arrays-test.js → tests/unit/model/native-access/native-access-arrays-test.js

@@ -39,7 +39,7 @@ class TestSchema extends DefaultSchema {
 }
 
 if (CUSTOM_MODEL_CLASS && HAS_NATIVE_PROXY) {
-  module(`unit/model/native-access-arrays`, function (hooks) {
+  module(`unit/model/native-access/native-access-arrays`, function (hooks) {
     setupTest(hooks);
 
     hooks.beforeEach(function () {

tests/unit/model/native-access-test.js → tests/unit/model/native-access/native-access-test.js

@@ -21,7 +21,7 @@ if (CUSTOM_MODEL_CLASS && HAS_NATIVE_PROXY) {
     }
   }
 
-  module('unit/model/native-access', function (hooks) {
+  module('unit/model/native-access/native-access', function (hooks) {
     setupTest(hooks);
 
     hooks.beforeEach(function () {

tests/unit/model/native-access/tracked-properties-test.js

@@ -0,0 +1,176 @@
+import { test, module } from 'qunit';
+import { setupRenderingTest } from 'ember-qunit';
+import DefaultSchema from 'ember-m3/services/m3-schema';
+import { CUSTOM_MODEL_CLASS } from 'ember-m3/-infra/features';
+import HAS_NATIVE_PROXY from 'ember-m3/utils/has-native-proxy';
+import Component from '@glimmer/component';
+import hbs from 'htmlbars-inline-precompile';
+import { render, settled } from '@ember/test-helpers';
+
+if (CUSTOM_MODEL_CLASS && HAS_NATIVE_PROXY) {
+  class TestSchema extends DefaultSchema {
+    includesModel(modelName) {
+      return /^com.example.bookstore\./i.test(modelName);
+    }
+    setAttribute(modelName, attr, value, schemaInterface) {
+      schemaInterface.setAttr(attr, value);
+    }
+    useNativeProperties() {
+      return true;
+    }
+  }
+
+  module('unit/model/native-access/tracked-properties', function (hooks) {
+    setupRenderingTest(hooks);
+
+    hooks.beforeEach(function () {
+      this.store = this.owner.lookup('service:store');
+    });
+
+    test('Component getters which depend on m3 native properties update correctly', async function (assert) {
+      this.owner.register('service:m3-schema', TestSchema);
+      this.owner.register(
+        'template:components/show-book',
+        hbs`<h1 class="title">{{this.formattedTitle}}</h1>
+    `
+      );
+
+      this.owner.register(
+        'component:show-book',
+        class ShowBookComponent extends Component {
+          constructor() {
+            super(...arguments);
+          }
+          get formattedTitle() {
+            return `Title: ${this.args.model.title}`;
+          }
+        }
+      );
+
+      this.store.push({
+        data: {
+          id: 'urn:li:book:1',
+          type: 'com.example.bookstore.Book',
+          attributes: {
+            title: 'How to Win Friends and Influence People',
+          },
+        },
+      });
+
+      let book = (this.book = this.store.peekRecord('com.example.bookstore.Book', 'urn:li:book:1'));
+      await render(hbs`
+        {{show-book model=this.book}}
+      `);
+
+      let renderedTitle = this.element.querySelector('.title').innerText;
+      assert.equal(
+        renderedTitle,
+        'Title: How to Win Friends and Influence People',
+        'component is rendered correctly'
+      );
+      book.title = 'New title';
+      await settled();
+
+      renderedTitle = this.element.querySelector('.title').innerText;
+      assert.equal(renderedTitle, 'Title: New title', 'component is updated correctly');
+    });
+
+    test('Template getters which depend on m3 native properties update correctly', async function (assert) {
+      this.owner.register('service:m3-schema', TestSchema);
+      this.owner.register(
+        'template:components/show-book',
+        hbs`<h1 class="title">{{@model.title}}</h1>
+    `
+      );
+
+      this.store.push({
+        data: {
+          id: 'urn:li:book:1',
+          type: 'com.example.bookstore.Book',
+          attributes: {
+            title: 'How to Win Friends and Influence People',
+          },
+        },
+      });
+
+      let book = (this.book = this.store.peekRecord('com.example.bookstore.Book', 'urn:li:book:1'));
+      await render(hbs`
+        {{show-book model=this.book}}
+      `);
+
+      let renderedTitle = this.element.querySelector('.title').innerText;
+      assert.equal(
+        renderedTitle,
+        'How to Win Friends and Influence People',
+        'template is rendered correctly'
+      );
+      book.title = 'New title';
+      await settled();
+
+      renderedTitle = this.element.querySelector('.title').innerText;
+      assert.equal(renderedTitle, 'New title', 'template is updated correctly');
+    });
+
+    test('Component getters which depend on m3 native properties update acrross nested models correctly', async function (assert) {
+      this.owner.register('service:m3-schema', TestSchema);
+      this.owner.register(
+        'template:components/show-book',
+        hbs`<h1 class="title">{{this.formattedTitle}}</h1>
+  `
+      );
+      class NestedSchema extends TestSchema {
+        computeAttribute(key, value, modelName, schemaInterface) {
+          if (key === 'chapter') {
+            return schemaInterface.nested({
+              attributes: value,
+            });
+          }
+          return value;
+        }
+      }
+
+      this.owner.register('service:m3-schema', NestedSchema);
+
+      this.owner.register(
+        'component:show-book',
+        class ShowBookComponent extends Component {
+          constructor() {
+            super(...arguments);
+          }
+          get formattedTitle() {
+            return `Title: ${this.args.model.chapter.title}`;
+          }
+        }
+      );
+
+      this.store.push({
+        data: {
+          id: 'urn:li:book:1',
+          type: 'com.example.bookstore.Book',
+          attributes: {
+            chapter: {
+              title: 'How to Win Friends and Influence People',
+            },
+          },
+        },
+      });
+
+      let book = (this.book = this.store.peekRecord('com.example.bookstore.Book', 'urn:li:book:1'));
+      await render(hbs`
+        {{show-book model=this.book}}
+      `);
+
+      let renderedTitle = this.element.querySelector('.title').innerText;
+      assert.equal(
+        renderedTitle,
+        'Title: How to Win Friends and Influence People',
+        'component is rendered correctly'
+      );
+      book.chapter.title = 'New title';
+      await settled();
+
+      renderedTitle = this.element.querySelector('.title').innerText;
+      assert.equal(renderedTitle, 'Title: New title', 'component is updated correctly');
+    });
+  });
+}