Repository navigation
Conversation
RNAirshipMessageView and RNAirshipEmbeddedView applied every prop on each updateProps call, so an unrelated prop change reloaded the message or re-applied the embedded config. Compare against oldProps first. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Ulrico972
approved these changes
Oct 26, 2026
oristanovic
reviewed
Oct 27, 2026
| const auto &newProps = *std::static_pointer_cast<const RNAirshipEmbeddedViewProps>(props); | ||
| self.config = [NSString stringWithUTF8String:newProps.config.c_str()]; | ||
| const auto *old = oldProps ? static_cast<const RNAirshipEmbeddedViewProps *>(oldProps.get()) : nullptr; | ||
| if (!old || old->config != newProps.config) { |
Contributor
There was a problem hiding this comment.
Fabric passes a null oldProps on Insert, so a cell recycling in a list still re-applies the config and re-publishes the embedded ID — the guard only covers in-place updates. Was in-place update the case you were after, or should re-insertion be covered too?
oristanovic
reviewed
Oct 27, 2026
| { | ||
| const auto &newProps = *std::static_pointer_cast<const RNAirshipMessageViewProps>(props); | ||
| self.messageID = [NSString stringWithUTF8String:newProps.messageId.c_str()]; | ||
| const auto *old = oldProps ? static_cast<const RNAirshipMessageViewProps *>(oldProps.get()) : nullptr; |
Contributor
There was a problem hiding this comment.
nit: old reads as a bare adjective next to newProps — oldViewProps would pair better.
This branch has not been deployed
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
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 do these changes do?
RNAirshipMessageViewandRNAirshipEmbeddedView(iOS Fabric components) now compare the new props againstoldPropsinupdatePropsand only applymessageId/configwhen the value actually changed. A missingoldPropsis treated as changed so the initial props still apply.Why are these changes necessary?
Both components applied every prop unconditionally. Any prop change reassigned
messageID(whose setter callsloadMessage), reloading the message and losing scroll position, and re-applied the embeddedconfig, re-publishing the embedded ID and selection. Android only invokes setters for changed props, so this brings iOS in line.How did you verify these changes?
Ran the example app on the iOS simulator with temporary
NSLogs in thesetMessageID/setConfigsetters and a temporary harness (not included) with buttons to change only a style prop or themessageId/ embedded ID:messageIdchange: message reloads.configchange: config re-applied.The harness used a fake message ID, so scroll position and spinner behavior were not checked against a real message.
Verification Screenshots:
N/A
Anything else a reviewer should know?
sizeToContentin the embedded view is still applied on every update; it was out of scope here.🤖 Generated with Claude Code