fix(toDebugString): adds better handling of cycle objects #10099
Conversation
kwypchlo
force-pushed
the
decycyle-objects-for-debug
branch
3 times, most recently
from
5000f0a to
8e49697
Compare
| * @returns {Boolean} | ||
| */ | ||
| function canContainCircularReference(object) { | ||
| return isObject(object) && Object.keys(object).length; |
There was a problem hiding this comment.
this is A) wrong (keys() only deals with own enumerable properties), and B) inefficient --- not very helpful
As discussed in angular#10085 this commit adds a function for replacing cycle references in object. It is recursive and it knows if the reference has been made in a straight lin e (sibling objects will be left but object referencing same object will be replaced). It also changes the replacement string to '...' because the older one could be mistaken for html tag by browsers.
kwypchlo
force-pushed
the
decycyle-objects-for-debug
branch
from
8e49697 to
86b2fc3
Compare
|
@caitp I've updated the line you were referring to, now it covers inherited properties and should be quicker I guess |
|
I've said before I'm really not keen on this approach at all, it's more effort than it's worth |
|
Well, the longer you put me down, the more I get convinced that you may be right with this one. The function is pretty complicated for a small percent of use cases. However, the part about replacing |
|
If petes okay with it, then alright, but honestly it's a really small problem to just output "something", even if it's not exactly what we wanted, like just fallback on Object.prototype.toString or something |
|
I don't feel there is much point in decycling the objects for this. |
As discussed in #10085 this commit adds a function for replacing cycle references in object. It is recursive and it knows if the reference has been made in a straight line (sibling objects will be left but object referencing same object will be replaced).
It also changes the replacement string to '...' because the older one could be mistaken for html tag by browsers.
@petebacondarwin could you look at this one? The original one I posted on #10085 was faulty because it couldn't handle arrays and objects like Date, Boolean, RegExp. This one is a little longer though, as far as I tested, safer.