ember-concurrency icon indicating copy to clipboard operation
ember-concurrency copied to clipboard

waitForProperty() doesn't work when awaiting for task.isIdle

Open dsafonov-grid opened this issue 2 years ago • 7 comments

Team, could you please knock me in a right direction.

I have composed the following example on twiddle: https://ember-twiddle.com/29aa8f11ea2208bd5f4095f7c12bfdcd?openFiles=templates.application%5C.hbs%2C

Why doesn't it work the way that is mentioned in docs: http://ember-concurrency.com/docs/events

Tasks seems start to finish only once UI is updated.

Thank you

dsafonov-grid avatar Jul 22 '22 17:07 dsafonov-grid

I simplified your example to this JSFiddle: https://ember-twiddle.com/cd2a22c33dad1f55a12f1994bac9e4ff?openFiles=templates.components.my-component%5C.hbs%2C

It does appear to be a real bug... i'm guessing something to do with recent versions of ember and how/whether/when watched properties fire observers.

machty avatar Jul 23 '22 23:07 machty

I would be curious what the behavior is in Ember 3.20+ too. IIRC there were some bugs with tracked and computed/observer interop between Ember 3.13 and 3.20, and unfortunately Ember Fiddle is capped at Ember 3.18

maxfierke avatar Jul 25 '22 14:07 maxfierke

@maxfierke @machty I tried ember-quickstart app locally, with

 "@glimmer/component": "^1.1.2",
    "@glimmer/tracking": "^1.1.2",
    ember-concurrency: 2.2.1,
       "ember-cli": "~3.28.5"

Example works perfectly just as should be according to documentation.

PS I realized that optional-features.json contain "default-async-observers": true. it is true by default in ember-quickstart, and it seems affect the behavior once I turn it to false and rebuild the project then tasks to acting weird again.

dsafonov-grid avatar Jul 25 '22 15:07 dsafonov-grid

Running into this now. From what I quickly gather:

  1. We use @tracked properties for tracking inner Task/TaskInstance state, including a tracked property for isIdle
  2. When the state of the task/instance changes, setState is called
  3. This constructs a new pojo of state, and then calls assignProperties, which is an alias of Object.assign, to assign it to all of the tracked properties
  4. the waitForProperty yieldable uses classic Ember observer to watch the property
  5. But I don't know if / don't think you can do a classic observe on a tracked property and expect to get notified when it changes.

I'll spend a little more time digging to see if there's a solution and I'll share what I find.

machty avatar Dec 28 '23 05:12 machty

@dsafonov-grid I wrote some tests to reproduce but couldn't get things to break, then I re-read your comment on async observers and when I set the behavior back to old school sync observers, the tests started to fail.

I think if this bug only happens with sync observers then I think we can close this is a wontfix.

machty avatar Dec 28 '23 06:12 machty

I ran into another case of waitForProperty hanging indefinitely, even with async observers enabled.

@tracked alert: Alert | null = null

get modalReady() {
  return !this.alert;
}

// in a task:
await waitForProperty(this, 'modalReady')

I found that when setting alert back to null (which would make modalReady true), it would remain stuck on the waitForProperty.

I solved this by splitting alert into separate getter and setter that set modalReady tracked property.

machty avatar Dec 31 '23 04:12 machty

Also for posterity I made a PollForProperty that does timeout polling: https://gist.github.com/machty/50be480319857b2513cb8b65f7433d68

I didn't end up needing it but it did solve work as an alternative to waitForProperty.

machty avatar Dec 31 '23 04:12 machty