This differential implements CommValidationModule validateMessageTypes method.
Details
Details
Build the app. We don't have a way to call this method yet.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
native/cpp/CommonCpp/NativeModules/CommConstants.cpp | ||
---|---|---|
20 ↗ | (On Diff #27441) | NOTE: The lack of caching here is intentional. jsi::Array happens to have deleted copy constructor. Theoretically we could use std::move here safely since according to jsi.h (I looked om it in the XCode) the move constructor for jsi::Array is default (so it acts as copy constructor). In practice it is better not to reuse objects that were once moved. We could also keep jsi::Array as an instance variable and manually copy its items to newly created jsi::Array that is returned here but I don't think it would be significantly faster than to recreate this array from scratch. |
native/cpp/CommonCpp/NativeModules/CommConstants.cpp | ||
---|---|---|
10–12 ↗ | (On Diff #27441) | Doesn't this work? |
20 ↗ | (On Diff #27441) | Yeah, in this case it makes sense - NATIVE_MESSAGE_TYPES probably will be accessed once, during the check (correct me if I'm wrong). If we ever really need caching, we can do something that expo does for caching module objects (1, 2). Maybe it's worth adding a TODO comment or linear task? You can also try doing this right away if it doesn't take you too long It's also worth noting that caching JSI objects requires proper deallocation - the cache should be cleared in HostObject's destructor or otherwise the app will crash (e.g. on reload) |
native/cpp/CommonCpp/NativeModules/CommConstants.h | ||
11 ↗ | (On Diff #27441) | I think this can be static or outside the class, but up to you |