Do you mean just blocking until all hashes are complete (like it is currently) or something smarter (e.g. we send a request to the server to delay the state check)?

iOS emulator takes about 1ms per one thread and the speed should be O(n). In the case from the linear task where the state check took 53s, there was about 2000 threads, so logging in would take additional 2s

In D9262#272510, @atul wrote:

In D9262#272502, @ashoat wrote:

In the past, we would need to update websites-responders.js when adding a new field to Redux state. Now that Redux state for the web is fetched via an API call, is there a different place we should be updating?

Based on my reading of https://phab.comm.dev/D9145, the "new place" is web/redux/default-state.js which I did update in this diff.

In the past, we would need to update websites-responders.js when adding a new field to Redux state. Now that Redux state for the web is fetched via an API call, is there a different place we should be updating?

Thanks @michal, that all makes sense.

I’d like to make sure @atul takes a look from the performance angle. Including this in ThreadStore is risky... any changes to the hashes will force every selector that depends on ThreadStore to re-run. If everything is memoized correctly, the performance cost should be limited. But if not there's a risk of hurting client performance by forcing unnecessary renders, especially if the actions "trickle in" separate from other changes to ThreadStore. (I assume "trickling in" would at least be necessary for a fresh log-in, where calculating all the hashes right away would be prohibitively expensive.)

What is the point of the outer if here...?

Android failure is unrelated

Thanks for explaining!

Nope, this doesn't relate to any keyserver change

Can you create a GitHub issue on the Expo repo and link it here before landing?

This does not handle pending threads (more precisely sidebars created from a message)

In D9005#271891, @patryk wrote:

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.

One thing to note is that with ENG-4981, one of the short term solutions was to just disable the pin action for sidebar source messages.

Accepting to unblock; we can dedup everything and figure out the types later. This Linear comment will hopefully be helpful

Should we use isMessageSidebarSourceReactionEditOrPin for this instead, and extend it to include messageTypes.CHANGE_ROLE? We'll want to eventually unify all of these checks in one place (see here). That will probably be in the message specs, but that doesn't mean we'll delete isMessageSidebarSourceReactionEditOrPin – it can be a helper function that checks the message specs

1. Please extend the test plan to include testing a request where the password field is not supplied. deleteAccountRequestInputValidator takes a password: t.maybe(tPassword), and it's not clear to me if that will crash when password is not supplied (as opposed to password: undefined or something)
2. After this change, we no longer need a request passed to deleteAccount at all. While accountDeletionResponder will need to support a request for a while (for old clients), deleteAccount is downstream and should be abstracted from that
   • First, this will require some changes in this file so that request is no longer a param. See inline comments
   • Second, you'll want to update accountDeletionResponder to no longer pass request in

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.

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

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?

The diff title here should probably include the word "notification" somewhere :)

We shouldn't use entityTextToRawString directly. I'm not sure what the alternatives are here, and what the added complexity will be. Please investigate and get back to us

In D8944#263772, @rohan wrote:

I think it'll be useful to add some unit tests for this regex, alongside the regex in D8834

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

Adding @atul as a reviewer in case he has any comments about React performance here

Don't forget to create one for the input validator idea too!

There's code that exists for the search experience that handles finding a thread for a given user if it already exists, OR creating a "pending" thread if that thread doesn't exist yet.

Looks good, but would be great if @tomek could take a look, as he's more familiar with our Android notif code

Nice thank you!!

How does the code for search results currently handle this? I think there's a similar need there for getting a "pending or realized" thread for the viewer and a given user. The new selector here makes me wonder if you're reusing as much of that code as possible.

I can confirm that we're looking to do only prefix search for @-mentions of chats. We'll continue to do a "full text search" for in the "Search chats" experience as well as the message search experience. But the typeahead is meant to be prefix-only

Publishing now, but will wait on CI before landing

Thanks!!