[lib] Avoid creation new ChatThreadItem unnecessarily in getSearchResultsForEmptySearchText
ClosedPublic
Actions

Authored by ashoat on Nov 11 2024, 3:12 PM.

Details

Reviewers

tomek
angelika

Commits

rCOMM8a334ca7bcda: [lib] Avoid creation new ChatThreadItem unnecessarily in…

Summary

In the process of investigating ENG-5674, I noticed an issue here. I had implemented a caching layer for useFlattenedChatListData so that it would avoid creating a new ChatThreadItem when the underlying data for it had not changed in Redux. But the code here was recreating the ChatThreadItem on every render anyways, forcing a bunch of components to rerender (and values to be recalculated) unnecessarily.

While I don't have a good long-term solution for ENG-5674 (and I probably won't prioritize it anytime soon), I think it's always best to avoid constructing new objects when the existing object suffices.

Test Plan

Flow

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

ashoat created this revision.Nov 11 2024, 3:12 PM

Harbormaster completed remote builds in B32599: Diff 45732.Nov 11 2024, 3:30 PM

ashoat requested review of this revision.Nov 11 2024, 3:30 PM

tomek accepted this revision.Nov 12 2024, 1:59 AM

This revision is now accepted and ready to land.Nov 12 2024, 1:59 AM

Closed by commit rCOMM8a334ca7bcda: [lib] Avoid creation new ChatThreadItem unnecessarily in… (authored by ashoat). · Explain WhyNov 12 2024, 6:49 AM

This revision was automatically updated to reflect the committed changes.

ashoat added a commit: rCOMM8a334ca7bcda: [lib] Avoid creation new ChatThreadItem unnecessarily in….

Revision Contents
Changeset List

Path

Size

lib/

shared/

thread-utils.js

3 lines

Diff 45749

View Options

lib/shared/thread-utils.js

Show First 20 Lines • Show All 531 Lines • ▼ Show 20 Lines	return facebook::react::createPromiseAsJSIValue(
DatabaseManager::getQueryExecutor().beginTransaction();		DatabaseManager::getQueryExecutor().beginTransaction();
for (const auto &operation : *storeOpsPtr) {		for (const auto &operation : *storeOpsPtr) {
operation->execute();		operation->execute();
}		}
if (messages.size() > 0) {		if (messages.size() > 0) {
DatabaseManager::getQueryExecutor().addOutboundP2PMessages(		DatabaseManager::getQueryExecutor().addOutboundP2PMessages(
messages);		messages);
}		}
		DatabaseManager::getQueryExecutor().captureBackupLogs();
DatabaseManager::getQueryExecutor().commitTransaction();		DatabaseManager::getQueryExecutor().commitTransaction();
} catch (std::system_error &e) {		} catch (std::system_error &e) {
error = e.what();		error = e.what();
DatabaseManager::getQueryExecutor().rollbackTransaction();		DatabaseManager::getQueryExecutor().rollbackTransaction();
}		}
}		}

		if (!error.size()) {
		DatabaseManager::getQueryExecutor().triggerBackupFileUpload();
		}

this->jsInvoker_->invokeAsync([=]() {		this->jsInvoker_->invokeAsync([=]() {
if (error.size()) {		if (error.size()) {
promise->reject(error);		promise->reject(error);
} else {		} else {
promise->resolve(jsi::Value::undefined());		promise->resolve(jsi::Value::undefined());
}		}
});		});
};		};
▲ Show 20 Lines • Show All 2,995 Lines • Show Last 20 Lines

[lib] Avoid creation new ChatThreadItem unnecessarily in getSearchResultsForEmptySearchTextClosedPublicActions