Add accumulator.resolveTransformWith method

[paulmolluzzo]

This is a PR trying to solve the resolveTransformWith feature suggestion in #15. The issue says the spec is a WIP, so if this PR needs extra work or just gets rejected I understand! 😄

This implements the resolveTransformWith method by adding the method in the accumulator factory and then checking for a "resolved value" within FunctionReducer#resolve to shortcut any callback that follows the resolveTransformWith call to ensure that the acc.resolvedValue is not overwritten.

The reason this pattern is used is because FunctionReducer#resolve (essentially) promisifies and chains the callbacks, and escaping that chain requires a throw.

I've written tests to cover the new code, but noticed there are no tests for accumulator/factory, and didn't introduce any (that code is sorta covered through other tests anyway).

Any comments or feedback is welcome! If this solution is not acceptable for inclusion, please let me know where it went wrong. Thanks! 🙌 🙌 🙌

[YiningChen]

could you make this 2 lines please? 😄 got tripped up for a bit processing the double pipes and comma so close to each other

[paulmolluzzo]

This catch wasn't here before (and isn't really doing anything). If we want to remove the catch or throw an error or something else please LMK.

[raingerber]

Does resolveTransformWith work if you want to resolve with a falsey value like null or undefined?

[paulmolluzzo]

You can set a falsey value but there’s a bug. 😅

If a falsey value is set as the resolvedValue then it will be overwritten by any future function because the short circuit in resolve/reducer-function#resolve is checking the truthiness of resolvedValue.

I’ll add a fix for this and include tests to cover that scenario and the fix.

Great catch! 🙌

[paulmolluzzo]

Hopefully the new commit covers this case better 😄

23917bb

[raingerber]

I was actually looking at this issue before you opened your PR, so I'm curious what you think of my approach: master...raingerber:accumulator-resolve-api. Since the root problem is the fact that we can't break out of Promise.reduce, I wrote a chainReducers function that replaces Promise.reduce while letting us return early. If you call acc.resolveTo or acc.resolveWith inside any promise in the chain, it skips over the subsequent promises entirely.

EDIT: I also think we should all have a conversation about the original proposal before we merge any code. The issue was a WIP, so we never decided on a final API or even if we definitely want to implement this feature.

[acatl]

paul, really want to thank you for giving this a shot, I looked at the code, and I think you are on the right track for solving with some tweaks needed. Unfortunately, I still think the feature itself is a WIP, there are a couple of things I think need to be resolved on defining the api first before even attempting to resolve it.

I think it was an error on my part to place this as an issue in the first place or should have been more clear about this just being an idea and not a feature ready to be worked on.

That aside let me share some of the problems I see with this current API proposal I had made:

The idea is to have an API that allows us to prematurely resolve at different levels

1. Resolve at transform expression level
2. Resolve at entity level and resolve

We want two types of resolutions:

1. With a given value
2. Re-direct to another entity

One challenge I see is the naming of the API which is mostly around the fact that this api needs to be flexible enough to accommodate for all without compromising on naming conventions, and also not being too specific...

maybe a better api would be:

acc.resolveTo(value, scope) 
acc.resolveWith(entityId, scope) 

Where scope can be transform, entity, all? not sure of the names, but a way to indicate at what scope you want to resolve.

The second challenge is how to exit early from a reducer function with out disrupting the expected flow of a reducer function.

Initially I had thought about throwing an error, to exit a reducer, the issue with this is a developer might be thrown off by this unexpected pattern, and even more, if he used the resolve inside a promise, then it would never get to datapoint resolution mechanism.

the other idea is similar to what you did but relies on a more specific object. where we are able to 'detect' that object as soon as we hit the next reducer and internally do something with the value.

The issue I see with this is that if a developer does not 'exit' a reducer when the resolve is executed then he would get an unexpected behavior. I think this is not so bad if it's documented enough. but at the end can be interpreted as a bad API design which I'm wary about.

(acc) => {
   if(flag) {
     acc.resolveTo('foo', 'transform')
     // forgot to return from here
     // User will get unexpected behaviour.
   }
  return 'bar'
}

I'm going to park this PR and issue for now till we have a better solution and also till we see people really needing it.

[acatl]

this value needs to be more specific isResolved could be used by anyone else, this is one of the issues with this solution. Maybe using Symbol? not sure yet

[paulmolluzzo]

@acatl thanks for the thorough response! This is a challenging issue to solve in a way that developers can anticipate the results and work within the pattern easily, so a solid spec would be a great first step.