You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Due to impl of getDescendantsById() across Store and RecordSet - for a summary record, Store still delegates to RS to collect all descendants (including the record itself), but summary rec is not in RS, so its own ID lookup fails, returns undefined, and is not filtered out.
We should be returning [] from these getters - unless there's any expectation that summary records can carry children?
Considered simply early-out in the StoreRecord getters if isSummary but we should probably ensure the public Store.getDescendantsById() API also respects.
Could compact return from RecordSet.getDescendantsById() or iterate and conditionally push onto return. We should ensure whatever approach we take if performant.
Not a big bug but it did bite me in a master/detail view - was not expecting that undefined member.
The text was updated successfully, but these errors were encountered:
So really weird to me that the public methods on Store and Record return the record itself! That's not documented, and seems totally inconsistent with getAncestorsById and getChildrenById. Seems to me we ought to fix that (carefully).
At the very least document it, but given the incongruity of it I would argue for the "breaking" change. I wonder if this method gets a lot of public use
As commented on the PR - agree, might be a change that breaks some apps relying on this behavior, but really does seem like a fix and the right one at that. (I am not one of my own descendants.)
I acknowledge it's a bit much, but we could introduce selfAndDescendants / selfAndAllDescendants getters for convenience - [this, ...this.descendants]. That would provide a 1-1 replacement for the prior incorrect impl for an app that wanted it, and it does seem like a potentially common pattern w.r.t. record selection - i.e. "I want this one and everything underneath it".
Due to impl of
getDescendantsById()
acrossStore
andRecordSet
- for a summary record, Store still delegates to RS to collect all descendants (including the record itself), but summary rec is not in RS, so its own ID lookup fails, returnsundefined
, and is not filtered out.We should be returning
[]
from these getters - unless there's any expectation that summary records can carry children?Considered simply early-out in the StoreRecord getters if
isSummary
but we should probably ensure the publicStore.getDescendantsById()
API also respects.Could
compact
return fromRecordSet.getDescendantsById()
or iterate and conditionally push onto return. We should ensure whatever approach we take if performant.Not a big bug but it did bite me in a master/detail view - was not expecting that undefined member.
The text was updated successfully, but these errors were encountered: