Skip to content

Conversation

@HoonBaek
Copy link
Contributor

@HoonBaek HoonBaek commented Feb 6, 2024

Fix

  • Figure out error: Cannot delete property renderMessageof #<Object> when using therenderMessage` props

Issue

  • This error appears from the customer side while I try to use the props renderMessage of Channel
image

@HoonBaek HoonBaek requested review from bang9 and chohongm February 6, 2024 07:17
@HoonBaek HoonBaek self-assigned this Feb 6, 2024
Copy link
Contributor

@bang9 bang9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

const renderedMessage = useMemo(() => renderMessage?.(omitObjectProperties(props, ['renderMessage'])), [message, renderMessage]);

When viewed from a different perspective, are there any benefits to removing renderMessage from props? 🤔

@HoonBaek
Copy link
Contributor Author

HoonBaek commented Feb 6, 2024

@bang9 It looks a little bit weird that the parameter of the renderMessage has a renderMessage

@bang9
Copy link
Contributor

bang9 commented Feb 6, 2024

@bang9 It looks a little bit weird that the parameter of the renderMessage has a renderMessage

Yes, so we're actually handling it with Omit in terms of types.
type RenderMessageParamsType = Omit<MessageProps, 'renderMessage'>;

However, in the actual code, we're not just passing MessageProps but passing all the values of MessageViewProps, so I was wondering if removing only renderMessage would make a significant difference.

Copy link
Contributor

@chohongm chohongm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@HoonBaek HoonBaek merged commit 5aaa966 into main Feb 7, 2024
@HoonBaek HoonBaek deleted the fix/renderMessage-cannot-delete-object-property-error branch February 7, 2024 04:44
@bang9 bang9 removed the v3.11.0 label Feb 7, 2024
@chohongm chohongm added v3.11.0 and removed v3.10.2 labels Feb 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants