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

3.20.3 => 3.20.4 regression: Getter doesn't fire if child collection of tracked model is cleared #19139

Closed
samcic opened this issue Sep 17, 2020 · 6 comments
Labels

Comments

@samcic
Copy link

samcic commented Sep 17, 2020

🐞 Describe the Bug

If I'm tracking a model object with a hasMany list of items, I have a getter that uses those items to compute something else, and I clear those items, then the getter doesn't fire (but it does if I add items to the list).

Here's a quick video to describe it visually:

https://recordit.co/OeEey7Edrw

This issue manifests on 3.20.4 but not 3.20.3, hence I've reported it in this repo and not Ember Data. I've also confirmed the issue on the latest Beta build (3.22-beta-3).

I'm aware of various other similarly-sounding reports of issues between 3.20.3 and 3.20.4 (e.g. this one on sort, and this one which seems rather similar but is apparently fixed in 3.22-beta-3). I don't believe there's an open issue tracking this particular behavior.

🔬 Minimal Reproduction

Describe steps to reproduce. If possible, please, share a link with a minimal reproduction.

  • git clone https://github.com/samcic/reactivity-bug.git
  • cd reactivity-bug
  • yarn install
  • ember serve
  • Navigate to http://localhost:4200/
  • Click the "Add" button to add some items to the collection and watch the counter increase
  • Click the "Clear" button to clear the items collection and watch the counter do nothing => BUG!
  • Click the "Add" button again to add another item, and watch the counter increase to 1 (i.e. the collection was cleared but the getter didn't react to the "cleared" state).

😕 Actual Behavior

When the collection is cleared, the getter itemsLength doesn't fire and the template doesn't update.

🤔 Expected Behavior

When the collection is cleared, the getter itemsLength should fire and the template should show the count 0.

🌍 Environment

  • Ember: 3.20.4 to current (issue doesn't exist in 3.20.3)
  • Node.js/npm: 12.14.0
  • OS: Windows 10
  • Browser: Chrome 85

➕ Additional Context

Add any other context about the problem here.

@pzuraq
Copy link
Contributor

pzuraq commented Sep 17, 2020

This is a bug in Ember Data. We were tracking too many things in Ember.get, and it had a high negative impact on performance. The things we were tracking were really the responsibility of EmberArray implementations to do, so we should fix the issue in Data.

I haven't had time to look into where this should happen, but essentially any time objectAt is called for an EmberArray, get(this, '[]') should also be called. This will properly entangle the state of the EmberArray.

@samcic
Copy link
Author

samcic commented Sep 17, 2020

@pzuraq Thanks for the lightning-fast response. I'll close this here and raise it again in Ember-Data if I can't see any existing issue there tracking it.

@rwjblue
Copy link
Member

rwjblue commented Sep 23, 2020

Chatted with @igorT, @snewcomer, and @pzuraq during the Ember Data meeting today. I think we need to consider this an actual regression, and see about fixing in Ember Data. I totally agree with what @pzuraq said above (that folks implementing their own custom objectAt should ensure they entangle with get(this, '[]')).

@samcic - Would you mind opening a failing test case for customers of EmberArray / MutableArray?

@igorT
Copy link
Member

igorT commented Sep 30, 2020

@snewcomer is working on writing up the test today

@snewcomer
Copy link
Contributor

#19164

@rwjblue
Copy link
Member

rwjblue commented Nov 13, 2020

This should be fixed and released in 3.20.6 and 3.22.1.

@rwjblue rwjblue closed this as completed Nov 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants