smart-queue next iteration WIP

[calumpeak]

WORK IN PROGRESS

Wanted to ping this up to get some feedback on the direction that this is going (adapting @prsn implementation) and continue to work on with feedback. I've not completely tested this yet so YMMV.

Addressing redux-offline/queue#4 And #4

Changes:

• Correctly add new entities to queue in scenarios where a merge isn't possible
• Fix immutability issues
• Allow merge strategy to be provided in config
• Remove error throwing and use console.error/warn to avoid breaking middleware
• Update merging strategies for CRUD updates as per addressed issues

TODO:

• Tests
• Types/Flow
• Merging strategy for base use cases needs full implementation to support optional fields (thinking I might use lodash.get to simplify this) and body/json types on effect

[prsn]

Hi @echoes221 ,
The way you have structured the code base it's an amazing work man.

[calumpeak]

@prsn thanks man :) Glad you like 👍

[sorodrigo]

Thanks for the great work I haven't had the chance to take a look into this carefully. I will try tomorrow!

[calumpeak]

Updated majority of tests, only one missing now is for the merge strategy.

[sorodrigo]

I think the delete1 shouldn't be in the outbox. If we remove the create1 there's no point on performing a delete1 on our backend

[calumpeak]

It's a little confusing, delete1 is Id 4 as per the constant setup, the delete coming in is ID 1. It's making sure that we don't override another delete. Maybe it's worth updating the constants to reflect their ID?

[sorodrigo]

Ok looks amazing great job!! I wonder have you thought about what to do with the merge function TODOs? Maybe I can help you test the remaining fn, I just need to know how to handle the TODOs.

[calumpeak]

@sorodrigo I'm glad you like!
I've got most of the types done, I'm just having some difficulty with with the callableObject type and how to reference that correctly - I might have to make the QueueFuncs IIFE to type them correctly.

The type:

export type QueueFunction = {
  (Outbox, OfflineAction, number, Config): Outbox,
  allowedMethods: Method[]
};

Using that is a bit awkward on a queue function:

// This errors
export const readInQueue: QueueFunction = (outbox, action, index, { merge }) =>
  replaceAtIndex(outbox, merge(outbox[index], action), index);
readInQueue.allowedMethods = [READ];

Errors because allowedMethods is not defined at that point. So I can either extend the function (Object.assign(func, { allowedMethods }), or wrap them both in an IIFE. Unless you've got a better insight into this?

I've had some difficulty with flow and my IDE, it's a little clunky and slow compared to my (limited) experience with TS. I would suggest to move to TS to prevent us needing to manually type the t.ds file also, but redux-offline core is in flow so I'll save that for another conversation/next version (especially since babel7 has TS support baked in) :D

Regarding the merge strategy any help would be appreciated. I've left a couple of TODO's on the function itself. It's more about handling the optional meta.offline keys, as well as providing support for effect.body and effect.json
I thought about using lodash.get as that would allow us to statically type paths, and provide defaults on the fly without tonnes of conditionals.

[calumpeak]

So I settled on the Object.assign method for typing the callable objects. Not as neat, but it works.

Just the merging strategy to solidify. I'll have a think on it to see if I can come up with a smart way of doing it.

[calumpeak]

indexOfAction should also contain the check for methodAllowed as certain actions cannot be squashed over each other. We should aim to find the best available, not give up on the first id match which may not allow a merge strategy to take place. E.g. enqueue([Read, Update], Update) -> [Read, Update, Update], first check fails so we add to the queue as new entity, where what we actually want is [Read, Update]