-
Notifications
You must be signed in to change notification settings - Fork 143
Disconnect events during unmount #282
base: master
Are you sure you want to change the base?
Disconnect events during unmount #282
Conversation
Change RobloxRenderer to disconnect all events during unmount
@@ -541,6 +542,25 @@ return function() | |||
expect(spyRef.callCount).to.equal(2) | |||
spyRef:assertCalledWith(nil) | |||
end) | |||
|
|||
itSKIP("should not process events when unmounting", function() |
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.
Should this still be skipped? If it's not currently passing, should it get to passing before being merged?
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 am not sure what I should do with this, because the test passes when it's running in studio, but not using Lemur, because it does not simulate the ChildRemoved event. So I don't know if I should implement the feature in Lemur or if there is another way to test this!
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.
@ZoteTheMighty how should we handle this?
CLA Signature Action: Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you read and sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just by adding a comment to this pull request with this exact sentence:
By commenting with the above message you are agreeing to the terms of the CLA. Your account will be recorded as agreeing to our CLA so you don't need to sign it again for future contributions to our company's repositories. 0 out of 1 committers have signed the CLA. |
Closes #235.
Change RobloxRenderer to disconnect all events during unmount, by adding a method
disconnectAll
to the SingleEventManager.The test I added to RobloxRenderer is currently skipped, because lemur does not have the
ChildRemoved
event implemented. However, the test passes when testing in studio. Let me know if I should go implement lemur or if you have any other ideas about how I could test this.Checklist before submitting:
CHANGELOG.md