fix: remove dangling variable references on model destroy #165
Conversation
This comment has been minimized.
This comment has been minimized.
Codecov Report
@@ Coverage Diff @@
## main #165 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 52 53 +1
Lines 1429 1445 +16
Branches 194 195 +1
=========================================
+ Hits 1429 1445 +16
Continue to review full report at Codecov.
|
This comment has been minimized.
This comment has been minimized.
| @@ -87,6 +89,10 @@ export class VariableManager { | |||
| referenceMap.set(location.toString(), reference); | |||
| } | |||
|
|
|||
| this.beforeModelDestroyedEvent | |||
There was a problem hiding this comment.
No need to unsubscribe here because the observable gets completed with the take(1), correct?
There was a problem hiding this comment.
Correct - this observable emits exactly once, right before the destruction. This does remind me though - the reference may no longer exist at that point, so let me ensure that case doesn't error out.
There was a problem hiding this comment.
Added a test case and handled this situation (it was logging but not erroring - now it cleans up the subscription in this case, so it never outlasts its potential utility).
This comment has been minimized.
This comment has been minimized.
|
🎉 This PR is included in version 1.2.1 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
Description
This change fixes a bug where dangling references are left over when a subtree of a model tree is destroyed. A later update of the variable would previously have attempted to access a destroyed model when updated. To fix this, we create a new lifecycle event that fires before a model is destroyed, allowing any defunct references to be cleaned up.
Testing
Added more unit tests, and updated existing ones.
Checklist: