BelongsTo PromiseProxy unchanged from null

[jlami]

https://ember-twiddle.com/8699e6c887c9eb2cb4344fb1d2551dbe?openFiles=templates.application.hbs%2C

The problem exists with ember-data 3.2+ and power-select

It seems that I can fix it by changing one line in _updateLoadingPromise in 3.3: https://github.com/emberjs/data/blob/master/addon/-legacy-private/system/relationships/state/relationship.js#L631

if (this._promiseProxy) { to if (this._promiseProxy && this._promiseProxy.get('isPending')) {

Which makes sure that the resulting object from getData is updated when changed and not just the .content part is changed.

https://github.com/cibernox/ember-power-select/blob/master/addon/templates/components/power-select/trigger.hbs#L1 This if directly checks the belongsTo attribute, and thus uses the result from getData. This starts at null for an empty record. But setting it to anything else will not change the return value and thus not update anything.

[runspired]

@jlami I believe this is fixed by #5562 which I have opened #5579 to track the backporting for. You can test now by testing canary (master branch).

[jlami]

I just tested canary and it does not fix this issue. Too bad ember-twiddle does not work with canary (or beta) at the moment.

I could try making a failing test for this.

--edit

I think the problem here is that while the property does fire a change event, if you use it as a parameter to a component the value is checked versus the old value. So only if the getData call will actually return a new object will the component get an actual update. So this might actually also be a 'glimmer/component problem' since it kind of ignores the change event? But I guess it is easier to fix here.

[runspired]

if (this._promiseProxy && this._promiseProxy.get('isPending')) {

ftr we've never officially supported adding still-initially-loading records to relationships, and I don't believe we intend on doing so now. This may not be what is happening here but the isPending check being a fix makes me suspicious that it is.

[runspired]

The twiddle provided works when ember-data 3.4 is used, and tests fail to replicate this issue. Closing unless a better reproduction is provided.

[jlami]

ftr we've never officially supported adding still-initially-loading records to relationships, and I don't believe we intend on doing so now.

It was actually to make sure it works when the promise has already settled. In that case the proxy can already have been used/read. So making sure the else is taken there will make sure the proxy will be recreated.

The twiddle provided works when ember-data 3.4 is used, and tests fail to replicate this issue. Closing unless a better reproduction is provided.

I'm testing the twiddle offline now and have inconsistent results myself too. Will let you know when I figure out what is happening here.

[jlami]

It looks like I can trigger the bug again by using ember-power-select-blockless This is a very simple wrapper around ember-power-select that should not change much. It just translates a ember helper to a block version. Maybe this extra template confuses glimmer somehow to not pass the changed attribute to the final component?

The twiddle is updated to show the bug again.

--edit I think that my initial assessment of the problem is still correct. The belongsTo computed property returns the Proxy, and that reference should change. (At least when the promise has resolved once, but I'm not sure if changing a still pending promise makes much sense)

Otherwise you can't build components that rely on attributes being set like ember-power-select does here: https://github.com/cibernox/ember-power-select/blob/296c978a8e23144b54617d8bb385ee3852a7b6ce/addon/components/power-select.js#L147 The wrapping component ember-power-select-blockless does get a didUpdateAttrs, but then Glimmer checks if the attributes to ember-power-select have changed. And does this by checking the reference, not the content of the proxy. So it never sends new attributes to the nested component.

I also think that observers on the proxy wont work. Listening to properties on the proxy does work, but that is one level deeper and not necessarily always the case like the scenario above shows.

BTW it works after the first choice because removeInternalModelFromOwn resets the _promiseProxy to null. But since in the bug we start with null as belongsTo removeInternalModelFromOwn is never called.

[runspired]

BTW it works after the first choice because removeInternalModelFromOwn resets the _promiseProxy to null. But since in the bug we start with null as belongsTo removeInternalModelFromOwn is never called.

That's a bug, but it might also point me at the real bug

[jlami]

I don't really see how that is a bug. Should it not be possible to observe a proxied object and be able to see changes a few components down? I don't see how that would be possible without actually special typing the observing code to listen to the .content part of the proxy. Which I think defeats the purpose of the proxy.

If you want I can build a simpler twiddle that shows that didUpdateAttrs is not called two components deep. Maybe it is by design? But then I don't know how addons should make code that can work with sync and async belongsTo parameters as well as POJO's or strings cleanly.

[runspired]

Posting this comment here for visibility: https://github.com/emberjs/data/pull/5584#issuecomment-417140955

[runspired]

Closing this as ultimately the issue here does not seem to be with ember-data but an issue with how ember-power-select chose to listen for updates not being compatible with Ember's current behavior for determining whether didReceiveAttrs and didUpdateAttrs should trigger.

Proxies, by nature, are meant to be a stable wrapper around the content they proxy to. In the case of power-select, the library had accidentally come to rely on that proxy being torn down and re-created in situations it should not have been.

There are a few ways that power-select and/or users of power-select can resolve this:

• use a hook that does trigger (willRender)
• install a computed property that watches the dependent keys appropriately, or watches content of PromiseProxies to recompute.
• resolve async relationships before passing them into the component, instead of relying on power-select to unwrap the PromiseProxy

[jlami]

I don't entirely agree that this is outside the scope of ember-data. While it can be a design decision to make proxies stable, it is just that: a decision. As far as I know there is nothing inherently stable about proxies. While it might be good to try to keep the amount of changes to a minimum (less memory and less property changes), I don't see why one change per actual data change would be bad.

Power select is probably not alone in expecting belongsTo to change. It is just the first that brings this to light in a more obvious way. I think this change will break a lot more code. And it feels bad to just say that it is their own fault for using something the way normal Ember usage would make you expect it to work. I can't even find anything related to Proxies being stable in the documentation. So how would anybody be expected to know this?

I for one have built computed properties that only want to know whether a belongsTo has been set or not. If this code where used inside a component 2 levels deep I would run into the same problem.

• The willRender will not fix this. If you change my twiddle to use that, you will see that it only fixes one instance of the 2-level component usage. It is not a global solution, just one that might work in some situations.
• Having to change this code to explicitly mention the .content part of the Proxy is in my mind not a good solution. The Proxy is there to help prevent code like that.
• Resolving async relationships again goes against my DRY impulse. But might actually not be bad practice, since this will also help catch rejections. But it still is not an easy way to work with something (the Proxy) that is in my mind is created to make life simpler.

A better alternative in my mind might be to make all of ember-data Promise based. Also the sync part. But I don't think that is in the cards. (While you could probably make the same setup where the promise returned by 'belongsTo' would stay stable longer than I would want. It is probably more natural there to make code that will return different promises when the belongsTo has changed content.)

Maybe we have run into a missing feature of Ember itself. It looks like object.notifyPropertyChange(key) will not solve this. We might need something like object.notifySelfChanged() to be able to let these notifications bubble. But I'm not too familiar with the ember-core to know if it would be easier to change the way components pass attributes to fix this. But it feels they behave correctly now: references that have not changed should not trigger an attribute to change.