Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

fix(toDebugString): adds better handling of cycle objects#10099

Closed
interstella-5555 wants to merge 1 commit into
angular:masterfrom
interstella-5555:decycyle-objects-for-debug
Closed

fix(toDebugString): adds better handling of cycle objects#10099
interstella-5555 wants to merge 1 commit into
angular:masterfrom
interstella-5555:decycyle-objects-for-debug

Conversation

@interstella-5555

Copy link
Copy Markdown
Contributor

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.

@interstella-5555
interstella-5555 force-pushed the decycyle-objects-for-debug branch 3 times, most recently from 5000f0a to 8e49697 Compare November 17, 2014 18:59
Comment thread src/stringify.js Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
@interstella-5555
interstella-5555 force-pushed the decycyle-objects-for-debug branch from 8e49697 to 86b2fc3 Compare November 17, 2014 19:20
@interstella-5555

Copy link
Copy Markdown
Contributor Author

@caitp I've updated the line you were referring to, now it covers inherited properties and should be quicker I guess

@caitp

caitp commented Nov 17, 2014

Copy link
Copy Markdown
Contributor

I've said before I'm really not keen on this approach at all, it's more effort than it's worth

@interstella-5555

Copy link
Copy Markdown
Contributor Author

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 <<already seen>> with ... should be fixed nevertheless so I prepared #10103 with only that thing in there.

@caitp

caitp commented Nov 17, 2014

Copy link
Copy Markdown
Contributor

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

@petebacondarwin

Copy link
Copy Markdown
Contributor

I don't feel there is much point in decycling the objects for this.
Going with #10103

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants