Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature Request: Merge upsert methods while offline #9

Open
wildhart opened this issue Aug 18, 2024 · 0 comments
Open

Feature Request: Merge upsert methods while offline #9

wildhart opened this issue Aug 18, 2024 · 0 comments

Comments

@wildhart
Copy link

wildhart commented Aug 18, 2024

I expect my user to make many minor changes to many documents while offline - frequent text edits, incrementing counters, etc.

This causes a lot of queued methods offline, which for this specific use-case ( upsert calls, but *see caveats below) I think could be merged into a single upsert call. This would speed up the sync process, reduce server and client load during syncing (each method call might trigger client subscriptions), and would also minimise idb storage.

Minimising idb storage is important, because I know from experience, particularly on iOS, that if the user doesn't have much phone storage, stuff can get randomly purged. Note that even in the simple example below, the text item could be of significant size...

As an example, let say I upsert an Item with this method (I'll use jam:method for simplicity):

const upsertItemMethod = createMethod({
  name: 'items.upsert',
  schema: ...
  async run(item) {
    item._id = item._id || Random.id();
    const res = await itemsCollection.upsertAsync(item._id, item)
    return item._id;
  },
};

Then my UI code has the following event handlers:

async onAddItem() {
  await upsertItemMethod({text: '', counter: 0);
},

async onTextBlur(item, text) {
  await upsertItemMethod({_id: itemId, text});
},

async onIncrementCounter(item) {
  await upsertItemMethod({_id: itemId, counter: item.counter + 1});
},

I can image a scenario in my own projects where 20+ documents are created offline, each with 10+ edits - so thats 200+ methods queued offline.

I am proposing that you provide a handy function to wrap this method, such that new calls to the method overwrite or are merged with the first queued call to the method.

Obviously this would not be the default behaviour. By hacking your idb store myself I could probably achieve this in my own code, but I think providing a built-in way to define such method could be a useful feature for users of this package.

Something like this:

import { createMergeableMethod } from 'meteor/jam:offline'

const upsertItemMergedMethod = createMergeableMethod(upsertItemMethod);

Then the createMergeableMethod handler:

  • returns a new function, with the same arguments as the provided method;
  • when called:
    • checks to see if a previous call to this method exists in idb.
    • If so, replaces/merges the parameters into it (* see below)
    • If not, calls the method (jam:method will automatically inject the method into idb if offline).

Caveat

Obvously this can only work without side-effects if the data within the document is self-contained, i.e. nothing else depends on the intermittent state of the document as it evolves. This would be up to the developer to determine if merging methods is safe for their use-case.

Merging vs Replacing?

Should the data in the new method call replace the initial one, or be merged with it? Replacing would necessitate that the entire, updated object is passed into every method call, and if the method is defined with multiple parameters, that they are all replaceable. If partial objects are to be supported (as per upsert) then they would need to be merged. Merging would probably limit the API to a method which only takes one object parameter, so the merging code can merge the properties of that object. Merging would be my preference.

If merging, then perhaps the API could be simplified even further, so that the method is created for the user, e.g.

import { createMergeableUpsertMethod } from 'meteor/jam:offline'

const upsertItemMergedMethod = createMergeableUpsertMethod(itemsCollection, {
  schema: ... // as per jam:method
  // or
  validate: ... // as per jam:method
});

What do you think? If you haven't got time I could possibly work on a PR, since if you don't then I'll have to do it in my own code anyway. If you think this is too complicated or not useful, then maybe I could do a simpler PR to expose some hooks/methods on your idb calls to make this easier for people to achieve in their own code?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant