Repository navigation
Conversation
… drops useChatMessageDomElements only applies a payload when it carries an element the hook does not store yet. A removal adds nothing, so its payload is always discarded: the hook keeps a strong reference to the detached element until the page unmounts and keeps returning it while the plugin still requests its id, which then throws on parentElement. The hook also reads an empty payload as the page unmounting, although the client sends one whenever none of the requested ids is rendered on a mounted page. Treat the client payload as authoritative: filter it by the ids this plugin requested and replace the page whenever the result differs, in either direction. This drops the empty-payload inference, and filtering keeps elements requested only by other plugins out of the store. The handler now reads the requested ids from a ref, so the listener is registered once and removed on unmount instead of being re-added on every state change.
The client soft-deletes chat messages by nulling message and recording deletedAt and deletedBy, but LoadedChatMessage declares message as string and has no deletion fields. A plugin cannot read deletedAt without a cast, and one trusting the message declaration crashes on the first deleted message. senderRole is also sent by the client but missing from the type. Declare message as string | null and add senderRole, deletedAt and deletedBy, matching what the client delivers, so plugins can tell a deleted message from a live one instead of inferring it from a null message.
The server commands sample selects its custom messages by metadata, which a soft delete leaves intact, so it keeps requesting the DOM element of a deleted message. The DOM element manipulation sample calls search on message, which is null for a deleted message, and throws. Skip messages with deletedAt set in both filters. Test it for truthiness rather than against null, so the samples keep working with clients that do not send the field yet.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
What does this PR do?
useChatMessageDomElementsnow applies removals. The hook only accepted a payload carrying an element it did not store yet; a removal adds nothing, so it was always discarded. The client payload is now treated as authoritative: it is filtered by the ids this plugin requested and replaces the page whenever the result differs, in either direction (compared by element identity, so a replaced node is picked up too). This also:messages: []as "page unmounted" although the client also sends it for a mounted page rendering none of the requested ids — an empty payload now simply empties the page;LoadedChatMessagetype:messageis nowstring | null(the soft delete nulls it), andsenderRole,deletedAtanddeletedByare declared, matching what the client delivers.sample-server-commands-pluginandsample-dom-element-manipulationskip deleted messages. The latter threw on deleted messages (message.searchonnull).Closes Issue(s)
Closes #293
Related: bigbluebutton/bigbluebutton#25812 (client side, exposes
deletedAt/deletedBy) — bigbluebutton/bigbluebutton#Motivation
After a message was deleted,
useChatMessageDomElementskept a strong reference to its detached element until the chat page unmounted, and kept returning it to a plugin that still requested the id —sample-server-commands-pluginthen threw aTypeErroronparentElement. A plugin also had no typed way to know a message was deleted, so it could not stop requesting it. With this PR the hook only answers "which requested elements are on screen now", anddeletedAtfromuseLoadedChatMessagesanswers whether a message was deleted:useChatMessageDomElementsuseLoadedChatMessagesdeletedAtsetMore
tests/unit/dom-element-manipulation/chat/message/hooks.test.ts. "drops an element the client stops sending while the id is still requested" and "stores only the elements this plugin requested" fail on the previous implementation and pass now. Both samples passtscand lint against the local build (scripts/publish-to-project-folder.sh).deletedAt/deletedByarrive asundefined; the samples testdeletedAtfor truthiness for that reason.message: string | nullmay break the compilation of plugins with strict null checks that use it as a plainstring. At runtime nothing changes — those plugins already receivenullfor deleted messages.