Path: csiph.com!.POSTED.204-73-239-109.cust.centrio.cz!not-for-mail From: Cezary Tomczyk Newsgroups: comp.lang.javascript Subject: Re: Object.defineProperties Date: Mon, 8 May 2017 13:17:50 +0200 Organization: csiph.com Internet News Service Message-ID: References: <1943190.NgBsaNRSFp@PointedEars.de> <1592229.TLkxdtWsSY@PointedEars.de> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Injection-Date: Mon, 8 May 2017 11:17:52 +0000 (UTC) Injection-Info: csiph.com; posting-host="204-73-239-109.cust.centrio.cz:109.239.73.204"; logging-data="21205"; mail-complaints-to="admin@kev009.com" User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 In-Reply-To: Xref: csiph.com comp.lang.javascript:33254 On 08/05/2017 00:50, Stefan Weiss wrote: > Cezary Tomczyk wrote: >> So, Object.assign does shallow copy, not deep copy. My goal , by having >> "extend" function in that case, is to do deep copy. >> >> If I do extend({}, o); then new object will have his own data and changes >> made on new object must not reflect on the source object. > > Okay, that makes sense. I thought you were saying that the action of > Object.assign() itself caused the source object to be modified. > >> function extend(target, source) { >> var output = target; >> >> function processProperty(key) { >> if (isTypeOf(source[key], 'object')) { >> output[key] = {}; >> extend(output[key], source[key]); >> } else if (isTypeOf(source[key], 'array')) { >> output[key] = source[key].slice(0); >> } else { >> output[key] = source[key]; >> } >> } >> >> Object.keys(source).forEach(processProperty); >> >> return output; >> } > > Without testing this myself, I can see a potential problem: an array in the > source object could contain objects as elements. Your extend() function does > a shallow copy on arrays, so modifying objects in the target array will > affect both the source and the target. Since my last post I rewrote it to: function extend(destination, source) { var property, src; function cloneArray(a) { return Object.assign({}, a); } for (property in source) { if (Object.prototype.hasOwnProperty.call(source, property)) { src = source[property]; if (isTypeOf(src, 'object')) { destination[property] = destination[property] || {}; extend(destination[property], src); } else if (Array.isArray(src)) { destination[property] = src.map(cloneArray); } else if (getTypeOf(src) === 'date') { destination[property] = new Date(src.valueOf()); } else if (getTypeOf(src) === 'regexp') { destination[property] = new RegExp(src); } else { destination[property] = source[property]; } } } return destination; } Of course, this doesn't cover all scenarios, but at least some major cases that I need. > Apart from that, I still don't quite see the use for the isTypeOf() > function... what's wrong with this: > > if (Array.isArray(source[key])) { > // clone array (and recurse) > } else if (typeof source[key] == "object" && source[key] !== null) { > // clone object (and recurse) > } else { > // copy scalar values (and null) > } There are even more cases that I've discovered while writing unit tests. I gave up and simplified to: var _toString = {}.toString; function getTypeOf(obj) { if (arguments.length !== 1) { return false; } return _toString.call(obj).slice(8, -1).toLowerCase(); } function isTypeOf(obj, typeToCheck) { var type; if (arguments.length < 2) { throw new TypeError('[isTypeOf] requires two arguments'); // The error message will have more precise data as Thomas suggested } type = typeToCheck.toLowerCase(); return getTypeOf(obj) === type; } > For the sake of completeness, I'd like to amend my earlier statement: > The `typeof` operator, `Array.isArray()`, *and strict comparisons* are > usually all that's needed. This would not be enough. For example, if you do typeof Date or RegExp object you will get "object" in result. And that is not what I want. Because of that I think using "toString" here is fine. Even, if there are edge cases, at the moment I think this is an acceptable solution. Unless there will be no better solution ;-) > Also for the sake of completeness, not all objects can be cloned 1:1 so that > the they are 100% independent and disconnected. Simple data container > objects should be fine, though. >> However, when I run it in PhantomJS 2.1.1 (Mac OS X) then it throws exception: >> >> "TypeError: Requested keys of a value that is not an object. in object.js >> (line 9)" >> >> While I understand that "source" is passed as a non-object then I can't find >> out what's causing it. At least debugging PhantomJS is a bit hard. > > Sorry, I don't have enough experience with PhantomJS to help with that. I > suggested it for my current project, but was rejected. Their mailing list Interesting, but I do not see more reliable way of executing tests in browser environment. I mean, you can use browser itself and execute manually, but that's not the point. I know also that there are some fake DOM solution for NodeJS, but I have no experience with that. Anyway, it's an off-topic subject :-) > may be worth a try: https://groups.google.com/forum/#!forum/phantomjs Thanks. I've handled the problem. -- Cezary Tomczyk http://www.ctomczyk.pl/ https://www.aslint.org/ - Accessibility validation tool