-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[BUGFIX release] fix destroyRecord followed by unloadRecord and impli… #5077
[BUGFIX release] fix destroyRecord followed by unloadRecord and impli… #5077
Conversation
…cit inverse relationships
let rel = this._implicitRelationships[key]; | ||
|
||
let implicitRelationships = this._implicitRelationships; | ||
this.__implicitRelationships = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels a little "insider baseball" to me but I don't have a better suggestion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bmac well, we are in the same class were it is private... Ya It's not amazing. Other ideas end up being junky too. I may circle back after this gets backported, and try to tidy this pattern up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think the basic approach is fine and i suspect it seems a bit iffy mainly because these data structures already have a lot of complicated state. I suspect landing the simplify relationships work will make this kind of resetting not look so bad
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
let rel = this._implicitRelationships[key]; | ||
|
||
let implicitRelationships = this._implicitRelationships; | ||
this.__implicitRelationships = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think the basic approach is fine and i suspect it seems a bit iffy mainly because these data structures already have a lot of complicated state. I suspect landing the simplify relationships work will make this kind of resetting not look so bad
…cit inverse relationships