neumino / rethinkdbdash

An advanced Node.js driver for RethinkDB with a connection pool, support for streams etc.
MIT License
848 stars 108 forks source link

`r.do` incorrectly requires 2 arguments #344

Closed aleclarson closed 7 years ago

aleclarson commented 7 years ago

These queries...

r.do([]).run()
r.do(r.expr(1)).run()

...throw this error:

ReqlDriverError: `r.do` takes at least 2 arguments, 1 provided

Even though r.do can be passed a single query or an array of queries (without a function argument).

aleclarson commented 7 years ago

Another symptom:

r.do(r.args([ r.expr(1), r.expr(2) ])).run()

...throws this error:

ReqlDriverError: `do` takes at least 1 argument, 0 provided
neumino commented 7 years ago

Hum, yea it's a bit weird to write r.do(1) but it's definitively different than the official driver.

neumino commented 7 years ago

Released in 2.3.31

neumino commented 7 years ago

Also note that r.do(r.args([1,2])) returns 1 and not 2, so I would probably not use r.do and r.args together.

aleclarson commented 7 years ago

Yeah those are just repros. 😄

Why the different behavior for r.do with r.args?

neumino commented 7 years ago

This is the annoying part of r.do.

r.do(1, 2, func) is sent as "do(func, 1, 2)". So when you write r.do(r.args([1,2,func]), we send "do(args(1,2,func))", which becomes do(1, 2, func) instead of do(func, 1, 2)

aleclarson commented 7 years ago

Why not check r.args for: typeof arguments[arguments.length - 1] === 'function' and move the function to the front if true?

Perhaps a special case inside r.do is necessary?

neumino commented 7 years ago

I don't think that's needed, the purpose of r.args is to make using a variadic method easier.

r.do(r.args([value1, value2]), function(args) { ... })) is probably what you'll write. I don't see a good reason why you would store the function with value1, value2 etc.

aleclarson commented 7 years ago

I see, good point! 👍