rtfeldman / seamless-immutable

Immutable data structures for JavaScript which are backwards-compatible with normal JS Arrays and Objects.
BSD 3-Clause "New" or "Revised" License
5.37k stars 195 forks source link

FIX #130: losing some properties #131

Closed tad-lispy closed 8 years ago

tad-lispy commented 8 years ago

This is a first take on the #130 issue.

There is an added check in addPropertyTo internal helper that will return early if property with given name is already defined on the target.

rtfeldman commented 8 years ago

Thanks for your work on this @lzrski!

Looking at this implementation, I think something like #81 is the right solution to #130. One problem with this approach is that you'll get opposite behavior in development builds compared to production builds; in development mode, your property gets added with no problem, but in production mode, it gets overridden! This is error-prone to say the least. 😉

81 seems to be the best solution to this (assuming it can work; seems like there might be issues with arrays, but not sure without experimenting), and that seems like the right path to go down here.

Thanks again!

tad-lispy commented 8 years ago

Didn't thought about development vs. production aspect before. I fully agree - #81 is the ultimate way. Thanks for taking time to look at it.