Loon and Firestore API Consolidation Proposal

[rayliverified]

Quick bit of feedback on an API proposal.

Proposal: Introduce a set method in Loon that behaves like Firestore's set, which can create or update a document without throwing an error if it already exists.
Example: Loon.collection('users').doc('1').set({'name': 'John', 'age': 28});

I was surprised that create fails if the document already exists. I looked for a parameter configuration option such as create(set: true) but didn't find one. Instead, there's createOrUpdate.

Loon seems to also be missing Firestore's set(merge: true) option which will merge maps and map values instead of overwriting the entire map, potentially deleting old keys.

I saw you mention that you're still thinking about the query API.

Other than that, Loon's API is fantastic. I like the approach you took with TypeAdapters and skipping it altogether along with skipping type registration like many other libraries.

[danReynolds]

Hey! Yea let's break this down.

I was surprised that create fails if the document already exists. I looked for a parameter configuration option such as create(set: true) but didn't find one. Instead, there's createOrUpdate.

Yea I wanted the semantics of create to only be about creating a new document in the store. createOrUpdate solves the use case where you aren't certain a document exists or not and just want it made or updated. I'd consider other names on it based on what's most familiar in the community, I know Firestore has the set API which would be a good name alternative, the name createOrUpdate was carried over from my past experience with libraries that named it that way.

Loon seems to also be missing Firestore's set(merge: true) option which will merge maps and map values instead of overwriting the entire map, potentially deleting old keys.

On this point, this library has definitely taken a lot of influence from Firestore. It's a collection document store after all and I originally wrote it to act as an alternative caching and reactive layer for some Firestore apps. That said, Firestore documents are always serializable map objects. Loon is a more generic store that can additionally store primitive values, or non-serializable objects since persistence is completely optional.

Given that, a set(merge: true) option wouldn't make sense in all scenarios, since the library wouldn't know how to merge those data types. The API could attempt to merge it if possible and replace it otherwise, but that could be unintuitive. For now, I'd recommend writing an extension method on the Document class like setJson or merge or something that does that for you. I'm open to other solutions, but I want the APIs to do what you'd expect for all data types.

I saw you mention that you're still thinking about the query API.

Yes for sure! I write a lot of code for myself and have surely made some weird assumptions when viewed by the larger community. I'm definitely down to change things based on feedback.

Other than that, Loon's API is fantastic. I like the approach you took with TypeAdapters and skipping it altogether along with skipping type registration like many other libraries.

💙

[rayliverified]

From my years of using Firestore, my takeaway is that to avoid confusion and "quirks", the API should be consolidated to a single function with param modifiers. What I would do is consolidate all of it to set() with params.

Interesting story, I use a Firestore cheat sheet to document all the Firestore quirks I find. The number of times I have to refer back to that sheet is much too high.

As a demonstration, try to answer this off the top of your head: What is the difference between Firestore's set(merge: true) and update()?

Check My Answer

This distinction has slipped my mind multiple times and I've caught many errors that were the result of incorrect uses of update. Instead, a single API set(insertIgnore: true) would eliminate the need for update altogether.

For querying, Firestore has a weird quirk with snapshot.data that perhaps you can avoid.

While migrating to Loon the past few days, an awkwardness I've run into is that Loon is missing the ability to set Lists of objects to a collection directly.

Currently, this is what I'm doing.

Loon.collection('achievements').doc('all').createOrUpdate(
  achievementsList.map((e) => e.toJson()).toList()
);

achievementsSubscription = Loon.collection('achievements')
  .doc('all').stream().listen((snapshot) {
  if (snapshot != null) {
    achievements = (snapshot.data as List)
      .map((json) => Achievement.fromJson(json))
      .toList();
  }
});

Instead, it would be nice to have something like:

Loon.collection('achievements')<Achievement>(
        'achievements',
        fromJson: Achievement.fromJson,
        toJson: (achievement) => achievement.toJson(),
      )
.set(
  achievementsList
);

For the id, there could be an id mapper. id: (achievement) => achievement.id.

One more thing is instead of subcollection, it would be nice to have the collection syntax.
collection().doc().collection().doc() instead of
collection().doc().subcollection().doc().

[rayliverified]

The accepted data signature could probably be loosened a bit to accept a List JSON instead of a map JSON.
Loon.collection('achievements').doc('all').createOrUpdate([List of Achievement Items JSON]).

type '_Map<String, dynamic>' is not a subtype of type 'Iterable'

Currently, I'm doing:

Loon.collection('user')
    .doc('home')
    .createOrUpdate({'items': homeItems});

DocumentSnapshot? document = Loon.collection('user').doc('home').get();
homeItems = List<dynamic>.from(document.data['items']);