jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/325344 )
Change subject: Don't use $.extend in nextState to preserve undefined fields
......................................................................
Don't use $.extend in nextState to preserve undefined fields
Since the state was cloned into an empty object with $.extend, undefined
properties were being deleted from it, not actually preserving those
keys. So:
nextState({hi: undefined, ho: 1}, {ho: 2})
//> {ho: 2}
It seems like a more consistent behavior would be to not lose any own
keys on the state as we do with the updates too, so that:
nextState({hi: undefined, ho: 1}, {ho: 2})
//> {hi: undefined, ho: 2}
Which is what this commit does by not using $.extend to clone the state
and instead just manually copy the keys to the new object, even the
undefined ones.
Change-Id: If4f2a3b0d25bb5ef34cfbc1f2c9c0b5479aeee9b
---
M resources/ext.popups/reducers.js
1 file changed, 11 insertions(+), 5 deletions(-)
Approvals:
jenkins-bot: Verified
Phuedx: Looks good to me, approved
diff --git a/resources/ext.popups/reducers.js b/resources/ext.popups/reducers.js
index cee40f7..c7bc80d 100644
--- a/resources/ext.popups/reducers.js
+++ b/resources/ext.popups/reducers.js
@@ -6,10 +6,10 @@
*
* N.B. OO.copy doesn't copy Element instances, whereas $.extend does.
* However, OO.copy does copy properties whose values are undefined or
null,
- * whereas $.extend doesn't. Since the state tree contains an Element
- * instance - the preview.activeLink property - we need to use
$.extend. But
- * to copy undefined/null into the state we need to manually iterate
over
- * updates and check with hasOwnProperty to copy over to the new state.
+ * whereas $.extend doesn't. Since the state tree contains an Element
instance
+ * - the preview.activeLink property - and we want to copy
undefined/null into
+ * the state we need to manually iterate over updates and check with
+ * hasOwnProperty to copy over to the new state.
*
* In [change listeners](/doc/change_listeners.md), for example, we
talk about
* the previous state and the current state (the `prevState` and `state`
@@ -22,9 +22,15 @@
* @return {Object}
*/
function nextState( state, updates ) {
- var result = $.extend( {}, state ),
+ var result = {},
key;
+ for ( key in state ) {
+ if ( state.hasOwnProperty( key ) &&
!updates.hasOwnProperty( key ) ) {
+ result[key] = state[key];
+ }
+ }
+
for ( key in updates ) {
if ( updates.hasOwnProperty( key ) ) {
result[key] = updates[key];
--
To view, visit https://gerrit.wikimedia.org/r/325344
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: If4f2a3b0d25bb5ef34cfbc1f2c9c0b5479aeee9b
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Popups
Gerrit-Branch: mpga
Gerrit-Owner: Jhernandez <[email protected]>
Gerrit-Reviewer: Phuedx <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
