- User Since
- Jul 20 2020, 9:28 AM (165 w, 5 d)
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
Thu, Sep 21
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.)
Wed, Sep 20
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)
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
- 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)
- 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
Tue, Sep 19
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
Adding @atul as a reviewer in case he has any comments about React performance here
Mon, Sep 18
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
Sat, Sep 16
Fri, Sep 15
Nice thank you!!
Thu, Sep 14
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
Wed, Sep 13