-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
The 'beforeSave' trigger breaks the dot notation for subdocuments (cf #567) #3912
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
Codecov Report
@@ Coverage Diff @@
## master #3912 +/- ##
==========================================
- Coverage 90.44% 90.43% -0.01%
==========================================
Files 114 114
Lines 7669 7682 +13
==========================================
+ Hits 6936 6947 +11
- Misses 733 735 +2
Continue to review full report at Codecov.
|
src/RestWrite.js
Outdated
|
|
||
| // Returns an updated copy of the object | ||
| RestWrite.prototype.buildUpdatedObject = function (extraData) { | ||
| var updatedObject = triggers.inflate(extraData, this.originalData); |
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.
please use const/let instead of var
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.
OK, done
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.
You apparently missed this one, @IlyaDiallo , no?
It seems you replaced the other ones but not 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.
I was thinking that the style requirement of let instead of var was only for declarations inside blocks, not at the beginning of a function. Is that wrong ?
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'm not sure I understand/can answer. @flovilmart, what do you say?
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 mean than this does not conform to the guideline because the var is visible outside the if block:
function f() { if(test) { var v; } }
... but this is (maybe) OK because var and let have the same scope:
function f() { var v; }
BTW is there a written styling guide for the project ?
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.
PS here it's in fact a const variable , so I've replaced var with const anyway, but the style question still stands.
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.
we use const for immutable variable (never reassigned) and let in place of vars as they behave more appropriately.
REST update / create queries setting values in subdocuments fail to update the values if a
beforeSavetrigger exists.Example: { "key.subkey" : value }
This is a fix for that issue.