calmm-js / partial.lenses

Partial lenses is a comprehensive, high-performance optics library for JavaScript
MIT License
915 stars 36 forks source link

Branches Does not preserve order #215

Open Millsky opened 5 years ago

Millsky commented 5 years ago

First off, I would like to say great work on this library, the documentation is fantastic and it makes dealing with complex objects easy.

My issue resolves around the order in which the branches are visited. for example:

L.branches(1,2,3) produces an object with the following keys. [1,2,3]. L.branches(3,2,1) produces an object with the following keys. [1,2,3]

I would expect L.branches(3,2,1) to visit the keys in the order in which they were provided as arguments, as opposed to the object key order.

I would propose using a Map to preserve the order of the arguments.

polytypic commented 5 years ago

Thanks! Just to recap, the current documentation says that

L.branches(p1, ..., pN) is equivalent to L.branch({[p1]: [], ..., [pN]: []})

which means that the keys are examined in object key order. The proposal is to change the order to match the order of the arguments instead.

:thinking:

Yeah, I think that change makes sense. It does likely require a bit more code to build the map and it is technically a breaking change, but unlikely to affect most users.

Did you run into a problem due to this?

I could imagine e.g. one using L.forEach with L.branches and being surprised due to the traversal order.

Millsky commented 5 years ago

@polytypic - I was using branches along with a disperse, I had a collection of ids to update.

L.branches(...formIds) the formIds were strings of numbers for example: ['1256', '9807', '123'].

Then I would use L.disperse(myBranchesTraversal, [1,2,3], {}); I guess this is another instance of JS behavior being a bit unexpected. As, when the string keys are put into an object they are sorted and then traversed in that order.

Another option may be to create another orderedBranches that maintains the argument or array order as opposed to the object key order, that way it wouldn't break any users that are relying on the object key order.

Thanks for the quick response by the way!