Page MenuHomePhabricator
Differential D6504

[native] Add some memoization to InnerRobotextMessage
ClosedPublic
Actions

Authored by ashoat on Feb 1 2023, 1:59 PM.
Tags
None
Referenced Files
F3503766: D6504.diff
Fri, Dec 20, 6:34 AM
Unknown Object (File)
Nov 18 2024, 7:25 PM
Unknown Object (File)
Nov 14 2024, 8:16 PM
Unknown Object (File)
Oct 29 2024, 4:21 PM
Unknown Object (File)
Oct 29 2024, 4:21 PM
Unknown Object (File)
Oct 29 2024, 4:20 PM
Unknown Object (File)
Oct 29 2024, 4:19 PM
Unknown Object (File)
Sep 26 2024, 8:39 AM
View All 82 Files
Subscribers
tomek

Details

Reviewers
atul
Commits
rCOMM2d527cea677d: [native] Add some memoization to InnerRobotextMessage
Summary
  1. Memoizing textParts will prevent their parent Text from rerendering when unnecessary
  2. Wrapping the whole component with React.memo brings it back to parity with the old PureComponent implementation
Test Plan

Flow, making sure robotext messages still render on native

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ashoat created this revision.Feb 1 2023, 1:59 PM
Herald added a subscriber: tomek. · View Herald TranscriptFeb 1 2023, 1:59 PM
ashoat added a child revision: D6505: [web][native] Always generate key from keyIndex in RobotextMessage.Feb 1 2023, 1:59 PM
Harbormaster completed remote builds in B15963: Diff 21799.Feb 1 2023, 2:15 PM
ashoat requested review of this revision.Feb 1 2023, 2:15 PM
atul accepted this revision.Feb 2 2023, 10:34 AM
This revision is now accepted and ready to land.Feb 2 2023, 10:34 AM
atul added inline comments.Feb 2 2023, 10:35 AM
native/chat/inner-robotext-message.react.js
152–159 ↗(On Diff #21799)

(I loosely remember something about exporting multiple things from a file with a React component could lead to issues with hot reloading or something?)

ashoat added inline comments.Feb 2 2023, 11:32 AM
native/chat/inner-robotext-message.react.js
152–159 ↗(On Diff #21799)

It's actually fine if both are React components – see here

atul added inline comments.Feb 2 2023, 11:33 AM
native/chat/inner-robotext-message.react.js
152–159 ↗(On Diff #21799)

Ah gotcha, thanks for clarifying + linking to that

Closed by commit rCOMM2d527cea677d: [native] Add some memoization to InnerRobotextMessage (authored by ashoat). · Explain WhyFeb 2 2023, 11:41 AM
This revision was automatically updated to reflect the committed changes.
ashoat added a commit: rCOMM2d527cea677d: [native] Add some memoization to InnerRobotextMessage.

Revision Contents
Changeset List

PathSize
native/
chat/
79 lines

Diff 21874

View Options

native/chat/inner-robotext-message.react.js

Loading...