Platform: Rename entity addAll to replaceAll

Created on 23 Jan 2020  ·  7Comments  ·  Source: ngrx/platform

Describe any alternatives/workarounds you're currently using

This is what addAll documentation says:

addAll: Replace current collection with provided collection

Then this really should be called replaceAll so people know their existing entities are being removed.

If accepted, I would be willing to submit a PR for this feature

[x] Yes (Assistance is provided if you need help submitting a pull request)
[ ] No

Accepting PRs Entity

Most helpful comment

Sounds good to me. Let's introduce the new one, and deprecate the old one.

All 7 comments

@alex-okrushko

Yeah, we had a case where addAll was used by accident instead of addMany and that lead to hours of debugging 😵

Maybe alias replaceAll to addAll and depreciate it?
removeAll can also be renamed to clearAll, but it's less error-prone, I think.

I don't know, i like addAll 😢 . Or at least don't use replaceAll but setAll instead, makes it easier to write 👍

👍 for setAll if we're going to rename this.

@brandonroberts @MikeRyanDev any thoughts on this? It wasn't the first time 🕵️‍♀️

Sounds good to me. Let's introduce the new one, and deprecate the old one.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

brandonroberts picture brandonroberts  ·  3Comments

sandangel picture sandangel  ·  3Comments

dollyshah-02-zz picture dollyshah-02-zz  ·  3Comments

doender picture doender  ·  3Comments

itprodavets picture itprodavets  ·  3Comments