Skip to content

Commit

Permalink
Fix tracked properties rerendering with m3 native properties
Browse files Browse the repository at this point in the history
  • Loading branch information
igorT committed Oct 28, 2021
1 parent ea879ca commit e9e39c0
Show file tree
Hide file tree
Showing 6 changed files with 387 additions and 19 deletions.
46 changes: 33 additions & 13 deletions addon/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,10 @@ let megamorphicModelProxyHandler, megamorphicNativeDeprecationHandler;

if (CUSTOM_MODEL_CLASS) {
const MegamorphicModelProxyHandler = class {
constructor() {
this.getting = '';
}

get(target, key, receiver) {
if (typeof key !== 'string' || key in target) {
return Reflect.get(target, key, receiver);
Expand All @@ -88,19 +92,35 @@ 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.

// `Ember.get` is going to check whether `receiver` has the `key`
// property already 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, and will have
// stashed our target object, and when checking for property existence
// will go directly to target, and because we have asserted that
// `key` is not in `target` we know we will not re-enter

// 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 (!DEBUG) {
if (this.getting === key) {
this.getting = undefined;
return undefined;
}
this.getting = key;
}
return get(receiver, key);
}

set(target, key, value, receiver) {
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
]
},
"dependencies": {
"@glimmer/component": "^1.0.4",
"babel-plugin-debug-macros": "^0.3.3",
"broccoli-funnel": "^3.0.8",
"ember-cli-babel": "^7.26.6",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down
140 changes: 140 additions & 0 deletions tests/unit/model/native-access/tracked-properties-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
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 rendered 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 rendered correctly');
});
});
}
Loading

0 comments on commit e9e39c0

Please sign in to comment.