Page MenuHomePhabricator

[native] introduce message searching to `CommCoreModule`
ClosedPublic

Authored by kamil on Jul 23 2024, 3:09 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 1, 7:41 PM
Unknown Object (File)
Tue, Oct 22, 9:38 PM
Unknown Object (File)
Tue, Oct 22, 9:27 PM
Unknown Object (File)
Tue, Oct 22, 3:37 PM
Unknown Object (File)
Tue, Oct 22, 2:44 PM
Unknown Object (File)
Tue, Oct 22, 1:36 PM
Unknown Object (File)
Mon, Oct 14, 4:12 PM
Unknown Object (File)
Mon, Oct 14, 4:11 PM
Subscribers

Details

Summary

The code on native to interface with search API using JSI.

Depends on D12852

Test Plan

Tested later in the stack by @inka.

Diff Detail

Repository
rCOMM Comm
Branch
cpp-search-2
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

kamil held this revision as a draft.
kamil published this revision for review.Jul 23 2024, 5:04 AM
tomek added inline comments.
native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp
2700

In some places, we're using a different syntax auto queryStr{query.utf8(rt)}. Which of them is the good practice?

native/schema/CommCoreModuleSchema.js
177

T[] syntax is discouraged and might be deprecated in the future https://flow.org/en/docs/types/arrays/#toc-array-type-shorthand-syntax

This revision is now accepted and ready to land.Jul 23 2024, 6:35 AM
native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp
2700

I am not sure, I think it's better to stick to explicit types unless auto really makes sense, e.g. the return type of a function might be changed later with something compatible or when the type is already easily readable from the right side of the assignment. In other cases, I think it decreases readability.

native/schema/CommCoreModuleSchema.js
177

going to add another diff and fix them all