mootools / mootools-core

MooTools Core Repository
https://mootools.net
2.65k stars 505 forks source link

Object.merge deletes EcmaScript Map Object #2805

Closed zortext closed 5 years ago

zortext commented 5 years ago

No Map object in the merged result. see fiddle: https://jsfiddle.net/970Lzu45/2

let A={
p1:'Param 1',
p2: 'Param 2',
}

let B={
p2: 'B param 2',
p3: 'B param 3',
map: new Map([['key1', 'value1'],['key2', 'value']])
}
let result=Object.merge(A,B);
DimitarChristoff commented 5 years ago

don't use -compat version. https://jsfiddle.net/970Lzu45/12/

zortext commented 5 years ago

The example returns an empty object instead of Map.

https://jsfiddle.net/970Lzu45/30/

Tested with latest version of Chrome 69.0.3497.100

DimitarChristoff commented 5 years ago

I guess. If you use Maps in your code, you're pretty confident you have a modern browser or good polyfills, you could use Object.assign() instead, merge loops only iterable properties and relies on clone for object like types or slice for arrays.

see https://github.com/mootools/mootools-core/blob/master/Source/Core/Core.js#L385-L412

as typeOf(new Map) returns an object... then we also see Object.clone(new Map(...)) returns an empty object.

the correct way to clone a Map is to return a new Map() but if it needs to be recursive... then the following needs to happen:

It's not a hard fix but if this is fixed, then a bunch of other parts would have to be made aware of new types and features. Not sure who'd have the time and energy to do all that...

zortext commented 5 years ago

Ended up not modifying Mootools and using an extra method to merge this type of data.

Beware if you use Maps in Mootools Class options. SetOptions use Object.merge()

Deep merge supporting Maps: https://gist.github.com/zortext/7e37142a91c9abfcc641f6e2dc306235 Maps will not be merged and will be keeped as they are.