lib/shared/avatar-utils.js
127	This is a confusing specification (name, list of params) for this function. Its real function is ENS resolution. It's given an `avatarInfo`, and if it finds `avatarInfo.type === 'ens'`, then it looks for the ENS avatar of the provided `userID` Several suggestions: Instead of `userID`, pass in `userInfo` or (even better) `ensAddress`. `UserAvatar` already fetches `userInfo` so this would avoid "fetching it twice", and more importantly it makes it clear what it's being used for (especially if it's `ensAddress`) We should swap the parameter order. The more important thing here is `avatarInfo`. Unless it's `{ +type: 'ens' }`, the other parameter is ignored We should rename the function to eg. `useResolvedAvatar` or `useENSResolvedAvatar`

This revision now requires changes to proceed.Mar 30 2023, 5:34 PM

ashoat mentioned this in D7262: [native] introude thread avatar component.Mar 30 2023, 5:35 PM

address ashoat's commentsy

ginsu added inline comments.Mar 30 2023, 10:59 PM

lib/shared/avatar-utils.js
129 ↗	(On Diff #24452)	Decided to use `userInfo` as the second parameter over `ethAddress` since we would have to copy and paste lines 131-139 in both `UserAvatar` and `ThreadAvatar` to get the `ethAddress`. If we still feel strongly about using `ethAddress` over `userInfo` I will make the necessary changes

Harbormaster completed remote builds in B17855: Diff 24452.Mar 30 2023, 11:10 PM

ginsu retitled this revision from [native/lib] introduce useAvatarForUserID hook in avatar-utils to [native/lib] introduce useENSResolvedAvatar hook in avatar-utils.Mar 30 2023, 11:18 PM

ashoat accepted this revision.Mar 31 2023, 5:30 AM

This revision is now accepted and ready to land.Mar 31 2023, 5:30 AM

rebase before landing

Harbormaster completed remote builds in B17892: Diff 24508.Mar 31 2023, 8:47 AM

Closed by commit rCOMMf87c4b812240: [native/lib] introduce useENSResolvedAvatar hook in avatar-utils (authored by ginsu). · Explain WhyMar 31 2023, 9:03 AM

This revision was automatically updated to reflect the committed changes.

ginsu added a commit: rCOMMf87c4b812240: [native/lib] introduce useENSResolvedAvatar hook in avatar-utils.