wikimedia/mediawiki-extensions-Popups
1
0
Fork 0
mirror of https://gerrit.wikimedia.org/r/mediawiki/extensions/Popups synced 2024-11-24 07:34:11 +00:00
Code Issues Packages Projects Releases Wiki Activity

Don't use $.extend in nextState to preserve undefined fields

Browse source 
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
This commit is contained in:
joakin 2016-12-05 20:16:36 +01:00
parent ec90fcba6f
commit ab4eff82e4
1 changed files with 11 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

16
resources/ext.popups/reducers.js
View file

@ -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];