chore: Deduplicate keyboard event in MessageBox #35481
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed changes (including videos or screenshots)
Whenever a callbackRef reference (e.g. pointer to the function) changes, the callbackRef runs again.
In the MessageBox component it is expected that the callbackRef's references can change at any given time, but attached event listeners are never removed, thus duplicating events in certain occasions.
In order to solve this issue in an elegant manner, a new hook is being introduced, called
useSafeCallbackRef. This hook allows acleanupcallback to be returned from the callbackRef, and will run such callback whenever the callbackRef is called after the first time. Thecleanupcallback is optional, in case some condition inside the callback is not required.If a callbackRef never requires a cleanup effect, there's no need to use it.
PS.: This PR is labaled as
chorebecause currently there are other safeguards around such duplicated events, so there's no behaviour change, and a changeset should not be added. This issue was noticed during the development of #32703 since those safeguards have changed, and breaks messaging behaviour.Also, having duplicate events is never a good idea, since it can lead to memory leaks.
Issue(s)
CORE-568
Steps to test or reproduce
No change in behaviour so far, only way to test is adding logs.
If you want to ensure proper behaviour, go to
apps/meteor/client/views/room/composer/messageBox/MessageBox.tsxand add a log tokeyboardEventHandler. Then open an encrypted room, evey keyboard input in the composer will output 2 logs to the console. This should not happen.Further comments
This is a niche issue, since usually callbackRefs do not change. Most of the time you should prefer to use react events.