search

machty / ember-concurrency

ember-concurrency is an Ember Addon that enables you to write concise, worry-free, cancelable, restartable, asynchronous tasks.
http://ember-concurrency.com
MIT License
690 stars 157 forks source link

outer task state incorrect when passed-in task is performed in component #336

Closed Turbo87 closed 4 years ago

Turbo87 commented 4 years ago

We hit what seems to be bug and created a minimal reproduction: https://github.com/Turbo87/__e-c-strangeness/

Our situation is that we have a task on a controller, that is being passed into a component. We then call .perform() inside the constructor of the component and we expected the .isRunning state to reflect that new state. Inside the component isRunning is changed to true, but the state of the task on the controller seems to never be updated 😱

I was assuming that if you pass a task to a component it would share the same instance instead of creating a copy, but it seem that that's not true 🤔

/cc @sdebarros @maxfierke @machty

maxfierke commented 4 years ago

I played around with this a bit, and it seems like it updates the controller after the task finishes, as expected, but you're right, it doesn't update the controller log directly on the start. It seems to be a problem for the initial run only, too. I expanded the example a little to be able to re-run the task, and if you perform the task again, it updates in both places as expected.

I also found that replacing the init call with didInsertElement, it works fine/as you'd expect. I can only imagine that this is some sort of under-the-hood Glimmer thing. Perhaps because it happens before/during the render in init, it does not notice the change? There's not much special going on within ember-concurrency here, other than a computed property and some this.sets of dependent properties.

Turbo87 commented 4 years ago

I've just tried the same thing with a Glimmer component (see https://github.com/Turbo87/__e-c-strangeness/commits/glimmer) and it's showing the same issue

/cc @pzuraq

Turbo87 commented 4 years ago

I've also tested this on different Ember versions (see https://github.com/Turbo87/__e-c-strangeness/commits/embers) and can confirm what @pzuraq said on Discord, that on 3.12 this would have hit the backtracking rerender assertion, but it seems that on 3.13 that assertion got broken and never warned us about this problematic behavior.