Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

await has issues with ember-data relationship promise proxies after 3.10 #54

Open
makepanic opened this issue Sep 6, 2019 · 4 comments

Comments

@makepanic
Copy link

makepanic commented Sep 6, 2019

Hi, thanks for this great addon.

I've upgraded our apps to ember-data >3.10 and found that the await helper stopped working when relationships change.

example construct:

{{model.rel.id}}
{{get (await model.rel) 'id'}}

If the rel relationship on model changes, await doesn't set the new value.
While debugging I found that the promise returned by e-d stays the same, but it's proxied value changes. The promise instance guid also stays the same.

Meaning the promise comparison will always return true:

https://github.com/fivetanley/ember-promise-helpers/blob/master/addon/helpers/await.js#L50

I'm unsure if this is a regression or some private api change in ember-data.

edit from discord:

  • this changed behavior is expected
  • Although we should likely switch the proxy out when the identity of the proxied content changes

@makepanic
Copy link
Author

makepanic commented Nov 6, 2019

@fivetanley I have a fix available locally.

It changes the behavior to not diff the promise instance but use its resolved value instead.
This means we always resolve the promise but only call recompute if its value changes.

Would you be ok with merging this change?

The change in await.js looks somewhat like this:

ensureLatestPromise(promise, cb) {
    cb.call(this, resolve(promise));
    return this.valueBeforeSettled;
  },
  setValue(value) {
    if (this.valueBeforeSettled !== value) {
      this.valueBeforeSettled = value;
      this.recompute();
    }
  }

This will break is-pending though. I'll check how to fix that. Maybe a combined approach would work, so we always resolve + value check for identical promise instance and still keep the promise instance check around.

edit:
Seems like keeping the old approach for is-pending works.
That means await with the change mentioned above will fix the ember-data issue.

@fivetanley
Copy link
Owner

@makepanic Sorry for the late reply. That sounds great if you want to open a PR for this change!

@veelci
Copy link

veelci commented Dec 1, 2023

@makepanic / @fivetanley I'm running into this issue as well with v.2.0.0 of ember-promise-helpers, v.3.28 of ember, and v3.28 of ember-data. I can reproduce my use-case by updating one of the tests in await-test.js. In general, the problem I'm seeing is that the value isn't updated because the promise is determined to be the same due to the Proxy.

  test('promises that get wrapped by RSVP.Promise.resolve still work correctly', async function (assert) {
    let deferred = RSVP.defer();
    let ObjectPromiseProxy = ObjectProxy.extend(PromiseProxyMixin);
    let proxy = ObjectPromiseProxy.create({
      promise: deferred.promise,
    });
    this.set('promise', proxy);
    await render(hbs`
      {{#with (await this.promise) as |obj|}}
        <span id="promise">{{obj.foo}}</span>
      {{/with}}
    `);
    deferred.resolve({ foo: 'hasAValue' });
    await afterRender(deferred.promise);
    assert.dom('#promise').hasText('hasAValue');

    deferred = RSVP.defer();
    proxy.promise = deferred.promise;
    deferred.resolve({ foo: 'hasAnotherValue' });
    await afterRender(deferred.promise);
    assert.dom('#promise').hasText('hasAnotherValue');
  });

veelci added a commit to aboveproperty/ember-promise-helpers that referenced this issue Dec 1, 2023
@veelci
Copy link

veelci commented Dec 1, 2023

I've pushed a PR w/ a test to fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants