Should `stamp/core/mergeOne()` handle `undefined` as its first parameter?

stampit-org / stamp

Stamps - better OOP model

https://stampit.js.org

MIT License

25 stars 3 forks source link

Should `stamp/core/mergeOne()` handle `undefined` as its first parameter? #55

Open PopGoesTheWza opened 4 years ago

PopGoesTheWza commented 4 years ago

Current implementation of @stamp/core/mergeOne() does not fully check if its first parameter dst is undefined. If the second parameter src is an object, dst is never checked and assumed to be an array/object and thus a TypeError: Cannot read property 'foo' of undefined is thrown.

stamp/packages/collision/index.js line 33

This issue is triggered by unit tests and standardiseDescriptor() (in packages/it/index.js lines:41, 47 and 56

This issue relates with #52

koresar commented 4 years ago

Yeah, you're right. It should handle the first undefined argument.

PopGoesTheWza commented 4 years ago

I can PR this (provided I get branching rights)

koresar commented 4 years ago

I think I have provided all the member the Write access to all stampit-org repos few minutes ago. Could you please check if I did it right?

PopGoesTheWza commented 4 years ago

It looks ok now. Thanks.