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

Allow fn passed to Async.fromNode to be partially applied #478

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions docs/src/pages/docs/crocks/Async.md
Original file line number Diff line number Diff line change
Expand Up @@ -379,8 +379,17 @@ As such, the need for binding may arise. `fromNode` provides a second, optional
argument that takes the context that will be used to bind the function being
wrapped.

Any curried interface will not be respected and if a curried interface is needed
then [`nAry`][nary] can be used.
The function returned from `fromNode` will be automatically curried, allowing you
to partially apply the function up to its penultimate parameter<sup>[1]</sup>, so long as the
arity of the original function given to `fromNode` can be determined via its `.length` property.

In practice this means that functions defined via `compose` or those making use
of `arguments`, spread args (`...args`) or default values for parameters, will not be
good candidates for partial application.

<sup>[1]</sup> The final parameter to the incoming function is provided to you by`fromNode` rather
than being part of the parameters that can be partially applied.


<!-- eslint-disable no-console -->
<!-- eslint-disable no-sequences -->
Expand Down
15 changes: 15 additions & 0 deletions src/Async/Async.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,21 @@ test('Async fromNode resolution', t => {
Async.fromNode(resCPS)(val).fork(rej(val), res(val))
})

test('Async fromNode partially applied', t => {
t.plan(2)

const val = 'super fun'

const rejCPS = (x, y, cf) => cf(x, y)
const resCPS = (x, y, cf) => cf(null, x, y)

const rej = y => x => t.equal(x, y, 'rejects an erred CPS')
const res = y => x => t.equal(x, y, 'resolves a good CPS')

Async.fromNode(rejCPS)(val)(val).fork(rej(val), res(val))
Async.fromNode(resCPS)(val)(val).fork(rej(val), res(val))
})

test('Async all', t => {
const all = bindFunc(Async.all)

Expand Down
12 changes: 10 additions & 2 deletions src/Async/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,22 @@ function fromNode(fn, ctx) {
throw new TypeError('Async.fromNode: CPS function required')
}

return (...args) =>
Async((reject, resolve) => {
const _fn = curry(fn)

return (...args) => {

if (args.length < _fn.length - 1) {
return fromNode(_fn(...args), ctx)
}
Comment on lines +55 to +57
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Per your documentation and our discussions their is a chance that the given function has a length of 0. Would this be a case to return an error?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding at this point is that if the function has a length of 0 then the safest thing to do is assume that it isn't being partially applied, and apply all of the parameters as per usual. This will mean that any code that currently passes a composed or ...args or argument function will continue to work in exactly the same way as it does today.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's true, sounds good to me


return Async((reject, resolve) => {
Comment on lines +51 to +59
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@evilsoft I know there is the issue with Async swallowing errors in one particular scenario, for this change, will wrapping this in the curry and the recursive call to fromNode create a similiar scenario?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nah, we should be good with this.
The Async "issue" is actually the desired behavior when dealing with Promises. And it is a behavior of Promises, so we should be good here!

fn.apply(ctx,
args.concat(
(err, data) => err ? reject(err) : resolve(data)
)
)
})
}
}

function fromPromise(fn) {
Expand Down