Update returns callback return value

integration-tests/tests/src/tests/sync/flexible.ts

@@ -62,10 +62,8 @@ function addSubscription<T>(
   options: Realm.App.Sync.SubscriptionOptions | undefined = undefined,
 ): AddSubscriptionResult<T> {
   const subs = realm.subscriptions;
-  let sub!: Realm.App.Sync.Subscription;
-
-  subs.update((mutableSubs) => {
-    sub = mutableSubs.add(query, options);
+  const sub = subs.update((mutableSubs) => {
+    return mutableSubs.add(query, options);
   });
 
   return { subs, sub, query };
@@ -282,12 +280,10 @@ describe.skipIf(environment.missingServer, "Flexible sync", function () {
 
         it("returns true if a subscription is added then removed", function (this: RealmContext) {
           const subs = this.realm.subscriptions;
-          let sub: Realm.App.Sync.Subscription;
-
           expect(subs.empty).to.be.true;
 
-          subs.update((mutableSubs) => {
-            sub = mutableSubs.add(this.realm.objects(FlexiblePersonSchema.name));
+          const sub = subs.update((mutableSubs) => {
+            return mutableSubs.add(this.realm.objects(FlexiblePersonSchema.name));
           });
 
           expect(subs.empty).to.be.false;
@@ -623,16 +619,16 @@ describe.skipIf(environment.missingServer, "Flexible sync", function () {
               });
             }).to.throw("mutableSubs.update is not a function");
           });
-        });
 
-        it("does not throw an error if a mutating method is called inside a update() callback", function (this: RealmContext) {
-          const subs = this.realm.subscriptions;
+          it("does not throw an error if a mutating method is called inside a update() callback", function (this: RealmContext) {
+            const subs = this.realm.subscriptions;
 
-          expect(() => {
-            subs.update((mutableSubs) => {
-              mutableSubs.add(this.realm.objects(FlexiblePersonSchema.name));
-            });
-          }).to.not.throw();
+            expect(() => {
+              subs.update((mutableSubs) => {
+                mutableSubs.add(this.realm.objects(FlexiblePersonSchema.name));
+              });
+            }).to.not.throw();
+          });
         });
 
         it("mutates the SubscriptionSet instance", function (this: RealmContext) {
@@ -737,6 +733,17 @@ describe.skipIf(environment.missingServer, "Flexible sync", function () {
 
           expect(subs).to.have.length(1);
         });
+
+        it("returns the return value of the update callback", function () {
+          const { subs } = addSubscriptionForPerson(this.realm);
+
+          const result = subs.update((mutableSubs) => {
+            return mutableSubs.add(this.realm.objects(FlexiblePersonSchema.name).filtered("age < 10"));
+          });
+
+          expect(result).to.be.an.instanceOf(Realm.App.Sync.Subscription);
+          expect(result.queryString).to.equal("age < 10");
+        });
       });
 
       describe("#add", function () {

src/js_subscriptions.hpp

@@ -369,12 +369,12 @@ void SubscriptionSetClass<T>::find_by_name(ContextType ctx, ObjectType this_obje
 
     auto it = subs->find(name);
 
-    if (it != subs->end()) {
-        auto sub = SubscriptionClass<T>::create_instance(ctx, *it);
-        return_value.set(sub);
+    if (it == subs->end()) {
+        return_value.set_null();
     }
     else {
-        return_value.set_null();
+        auto sub = SubscriptionClass<T>::create_instance(ctx, *it);
+        return_value.set(sub);
     }
 }
 
@@ -404,12 +404,12 @@ void SubscriptionSetClass<T>::find_by_query(ContextType ctx, ObjectType this_obj
 
     auto it = subs->find(query);
 
-    if (it != subs->end()) {
-        auto sub = SubscriptionClass<T>::create_instance(ctx, *it);
-        return_value.set(sub);
+    if (it == subs->end()) {
+        return_value.set_null();
     }
     else {
-        return_value.set_null();
+        auto sub = SubscriptionClass<T>::create_instance(ctx, *it);
+        return_value.set(sub);
     }
 }
 
@@ -551,7 +551,7 @@ MutableSubscriptionSetClass<T>::create_instance(ContextType ctx, realm::sync::Mu
  * @param object \ref ObjectType wrapping the SubscriptionSet
  * @param args \ref A single argument containing a callback which receives a mutable version of
  * the SubscriptionSet as its argument, and which updates the SubscriptionSet as required
- * @param return_value \ref None
+ * @param return_value \ref Returns the return value of the update callback
  */
 template <typename T>
 void SubscriptionSetClass<T>::update(ContextType ctx, ObjectType this_object, Arguments& args,
@@ -575,10 +575,12 @@ void SubscriptionSetClass<T>::update(ContextType ctx, ObjectType this_object, Ar
         auto mutable_subs_js = MutableSubscriptionSetClass<T>::create_instance(ctx, subs->make_mutable_copy());
         auto mutable_subs = get_internal<T, MutableSubscriptionSetClass<T>>(ctx, mutable_subs_js);
 
-        // Call the provided callback, passing in the mutable copy as an argument
+        // Call the provided callback, passing in the mutable copy as an argument,
+        // and return its return value
         ValueType arguments[]{mutable_subs_js};
         auto const& callback_return =
             Function<T>::callback(protected_ctx, protected_callback, protected_this, 1, arguments);
+        return_value.set(callback_return);
 
         // Commit the mutation, which downgrades its internal transaction to a read transaction
         // so no more changes can be made to it, and returns a new (immutable) SubscriptionSet
@@ -719,12 +721,12 @@ void MutableSubscriptionSetClass<T>::remove(ContextType ctx, ObjectType this_obj
     auto query = results->get_query();
 
     auto it = subs->find(query);
-    if (it != subs->end()) {
-        subs->erase(it);
-        return_value.set(true);
+    if (it == subs->end()) {
+        return_value.set(false);
     }
     else {
-        return_value.set(false);
+        subs->erase(it);
+        return_value.set(true);
     }
 }
 
@@ -758,12 +760,12 @@ void MutableSubscriptionSetClass<T>::remove_subscription(ContextType ctx, Object
         return sub.id() == sub_to_remove->id();
     });
 
-    if (it != subs->end()) {
-        subs->erase(it);
-        return_value.set(true);
+    if (it == subs->end()) {
+        return_value.set(false);
     }
     else {
-        return_value.set(false);
+        subs->erase(it);
+        return_value.set(true);
     }
 }
 

types/index.d.ts

@@ -816,8 +816,10 @@ declare namespace Realm {
              * @param callback A callback function which receives a {@link MutableSubscriptionSet}
              * instance as its only argument, which can be used to add or remove subscriptions from
              * the set.
+             *
+             * @returns The return value from the callback
              */
-            update: (callback: (mutableSubs: MutableSubscriptionSet) => void) => void;
+            update<ReturnValueType> (callback: (mutableSubs: MutableSubscriptionSet) => ReturnValueType): ReturnValueType;
         }
 
         const Subscriptions: {