[web] add std::optional (C++) <-> undefined (JS) marshalling
ClosedPublic
Actions

Authored by ashoat on Jul 14 2024, 6:06 PM.

Details

Reviewers

kamil
tomek
marcin

Commits

rCOMMd1c2b00e9d7c: [web] add std::optional (C++) <-> undefined (JS) marshalling

Summary

This follows a similar approach outlined in D8552. I was inspired by searching the string "BindingType<std::optional" on GitHub. This project provided a good implementation.

Depends on D12735

Test Plan

I tested this as part of the getLatestMessageEdit API I introduce later in this stack. I tested that on both web and iOS, which required using the std::optional <-> undefined marshalling

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

ashoat created this revision.Jul 14 2024, 6:06 PM

ashoat added a child revision: D12737: [SQLite] Introduce getRelatedMessages.

Harbormaster completed remote builds in B30309: Diff 42269.Jul 14 2024, 6:24 PM

ashoat requested review of this revision.Jul 14 2024, 6:24 PM

It's great to see these bindings! At some point, we should delete NullableString and NullableInt.

The diff looks reasonable but @kamil should also take a look.

This revision is now accepted and ready to land.Jul 15 2024, 1:46 AM

Making @kamil blocking based on @tomek's comment above.

In D12736#360718, @tomek wrote:

It's great to see these bindings! At some point, we should delete NullableString and NullableInt.

We should also be able to get rid of the web-specific getOlmPersistAccountDataWeb and just use getOlmPersistAccountData on web. I'll create a follow-up task for this before landing.

This revision now requires review to proceed.Jul 15 2024, 8:07 AM

I ended up not needing this, since I'm replacing the getLatestMessageEdit API with a getRelatedMessages API that returns an array.

That said, I think it's still work landing. Going to separate it out from the rest of my stack though.

ashoat removed a parent revision: D12735: [native] Introduce MessageEntity type alias in more places.Jul 15 2024, 4:46 PM

ashoat removed a child revision: D12737: [SQLite] Introduce getRelatedMessages.

ashoat added a child revision: D12737: [SQLite] Introduce getRelatedMessages.Jul 15 2024, 7:02 PM

ashoat removed a child revision: D12737: [SQLite] Introduce getRelatedMessages.

Awesome!

This revision is now accepted and ready to land.Jul 16 2024, 2:55 AM

We should also be able to get rid of the web-specific getOlmPersistAccountDataWeb and just use getOlmPersistAccountData on web. I'll create a follow-up task for this before landing.

Task with some context: ENG-5719.

Ah okay, no need to create a new follow-up task then – thanks for linking!

Closed by commit rCOMMd1c2b00e9d7c: [web] add std::optional (C++) <-> undefined (JS) marshalling (authored by ashoat). · Explain WhyJul 16 2024, 6:46 PM

This revision was automatically updated to reflect the committed changes.

ashoat added a commit: rCOMMd1c2b00e9d7c: [web] add std::optional (C++) <-> undefined (JS) marshalling.

kamil mentioned this in D13140: [web] Create a function that fetches messages in batches.Mon, Aug 26, 2:43 AM