In D8900#271640, @ashoat wrote:

Is the performance concern pointed out by @tomek going to lead to additional React render cycles? If yes, we should prioritize addressing it. cc @atul

Nice!

@kamil for changes related to data clearance. I didn't add him as a blocking reviewer since he might be absent for a longer time. But if he is back quickly I would appreciate his review on the data clearance.

This does not handle pending threads (more precisely sidebars created from a message). It's might be fine depending on the use case but I it feel like it should have a code comment in that case. Also agree with @tomek comments.

Overall looks good but I have a question. In the test plan you write:

const searchMessagesActionObj: KeyserverCall<
  [SearchMessagesRequest],
  SearchMessagesResponse,
> = {
  actionFunc: searchMessages,
  config: { keyserverSelection: 'fanout' },
};

but shouldn't the return type be [SearchMessagesResponse] as it's a fanout action?

I'm no longer using entityTextToRawString since I only need to extract alternative text. Please take a look at the latest diff, 31258. Throughout the entire stack, I use the useResolvedThreadInfo hook. Also, I'm not handling ENS names in this stack; user mentions are handled as they were before. The only change was made in the search index -> however this change does affect mentioning in the input bar only. I can add a comment that the new search index needs to be refactored to support ENS names.

I ended up adding a new setCommServicesAuthMetadata method because we will set user id, device id, and access token all at once initially, but subsequently may need to update just the access token.
also changed getCommServicesAccessToken to getCommServicesAuthMetadata since we will need to retrieve all three at once

address feedback

simplify

No reviewers on this

Thought about this some more... I suspect you haven't given much thought to ENS names, and to properly handle it will need updates across the whole stack, from the search index, to the display of results, to the representation of the "markdown", to the final rendering of the tag.

rebase and land

Okay, please make sure that upstream React Native does this on all builds too before landing!

(I think the profiler gets disabled on prod builds, I'll do a little more research)

In D9233#271796, @ashoat wrote:

It seems sketchy to do this in production, especially given it's undocumented. I'm worried about the potential impact on performance. Can we restrict this change to dev builds?

It seems sketchy to do this in production, especially given it's undocumented. I'm worried about the potential impact on performance. Can we restrict this change to dev builds?

Also prevent sidebar creation from a CHANGE_ROLE robotext. Context is in ENG-4981... this change won't solve the issue (since the client app crashes in the pending sidebar, before the sidebar creation can even go through) but it's probably a good idea

Is there a GitHub issue in Expo's GitHub repo for this?

Rebase before landing