-
Notifications
You must be signed in to change notification settings - Fork 241
fix: sanitizes extra dollar signs for operators #584
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
Conversation
lib/mongodb.js
Outdated
| while (spec.charAt(0) === '$') { | ||
| spec = spec.substr(1); | ||
| } | ||
| return spec; |
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.
The function can be simplified as follows:
function trimLeadingDollarSigns(spec) {
return spec.replace(/^(\$)+/, '');
}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.
Indeed! Applied.
jannyHou
left a comment
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, just a few suggestions.
| '$pop', | ||
| '$pullAll', | ||
| '$pull', | ||
| '$pushAll', |
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.
any reason deleting this?
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.
pushAll is deprecated and being removed from MongoDB 3.6: https://docs.mongodb.com/manual/release-notes/3.6-compatibility/index.html#remove-pushall-update-operator
f696d89 to
a978ae8
Compare
fixes #532
Add a helper function that removes all extra dollar signs.
e.g
$eq->eq,$$eq->eq( don't consider the case as an error).Checklist
👉 Read and sign the CLA (Contributor License Agreement) 👈
npm testpasses on your machine