-
Notifications
You must be signed in to change notification settings - Fork 364
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
types: support non-SQL styles of ds.execute
#1855
Conversation
289fa67
to
4622e36
Compare
7de30e5
to
961e7c5
Compare
ds.execute
ds.execute
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM in general 👍 a minor comment.
execute( | ||
collectionName: string, | ||
command: string, | ||
...parameters: any[], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: the execute
function in mongodb seems doesn't have a 3rd parameter, see
https://github.com/strongloop/loopback-connector-mongodb/blob/master/lib/mongodb.js#L467
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I appreciate your attention to details! 👍
MongoDB connector's execute
function does not describe additional parameters in the function signature, but uses arguments.slice
to access them:
// Get the parameters for the given command
const args = [].slice.call(arguments, 2);
This code was written before rest parameters were introduced to the language, therefore it was not possible to encode ...parameters
in the function signature.
Signed-off-by: Miroslav Bajtoš <[email protected]>
961e7c5
to
f13d50e
Compare
Add new overloads for
DataSource.execute
method to support non-SQL connectors like MongoDB.See loopbackio/loopback-next#3342 for background and https://loopback.io/doc/en/lb3/Executing-native-SQL.html for the existing documentation for
db.execute
API.Checklist
👉 Read and sign the CLA (Contributor License Agreement) 👈
npm test
passes on your machine