mohsen1 / json-formatter

Angular directive for collapsible JSON in HTML
http://azimi.me/json-formatter/demo/demo.html
Other
372 stars 85 forks source link

digest recursion problem #64

Open tysenmoore-xse opened 7 years ago

tysenmoore-xse commented 7 years ago

I have hit a digest recursion problem when opening an object that has more than 10 levels deep. For example,

testObj = { lvl0: { lvl1: { lvl2: { lvl3: { lvl4: { lvl5: { lvl6: { lvl7: { lvl8: { lvl9: { lvl10: { lvl11: { lvl12: { end:true } } } } } } } } } } } } } };

Then I attempt to open using:

<json-formatter json="testObj" open="14"></json-formatter>

Error:

Error: [$rootScope:infdig] 10 $digest() iterations reached. Aborting!
    Watchers fired in the last 5 iterations:...

I have tried various workarounds but cannot seem to get it right. Do you have suggestions or possibly a solution to the problem :-) ? I would like to avoid changing the digestTtl. Any suggestions welcome.

mohsen1 commented 7 years ago

Well, I don't expect this directive to trigger a digest for each level. Maybe something else is doing it. Can you try it in a isolated environment?

tysenmoore-xse commented 7 years ago

Thanks for the response. I have looked into this a fair amount yesterday trying to find the problem and work around it. I think what is going on is that the json-formatter directive is used recursively for each key in the object and all use the same open (set/change one and they are set/change). By setting the open to something > 10 every reference to open is initially set. As a result the isOpen is set/altered which then becomes dirty. I have not gotten to the bottom of this but it seems this recursive use of open/isOpen is causing this problem. I have tried changing the higher level API to use something other than open, but this really did nothing.

FWIW as a side note, if I use my branch which supports the open watch I find I can incrementally increase open one at a time to exceed 10. So a "one by one" increment of open seems to be fine. Strangely enough after the object reaches the maximum depth (e.g. 14) I can then dynamically change open to 1, then 14 without the crash. It really seems there is an initialization condition of something that is causing this problem. Once initialized it works fine--no recursion.

As another test, I took your master branch and used the demo to show the problem. I made the following modifications for testing:

demo.js changes: Modified the bower components to use CDN (would be nice if master contained this change):

<script src="https://cdnjs.cloudflare.com/ajax/libs/angular.js/1.6.1/angular.js"></script>
<script src="https://cdnjs.cloudflare.com/ajax/libs/angular.js/1.6.1/angular-sanitize.js"></script>
<link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/twitter-bootstrap/4.0.0-alpha.6/css/bootstrap.css">

Then added:

$scope.testObj = { lvl0: { lvl1: { lvl2: { lvl3: { lvl4: { lvl5: { lvl6: { lvl7: { lvl8: { lvl9: { lvl10: { lvl11: { lvl12: { end:true } } } } } } } } } } } } } };

demo.html changes: Added the displaying of testObj:

<h4>Deep JSON</h4>
<json-formatter json="testObj" open="100"></json-formatter>

When you run this in the browser (Chrome in my case) it will only show the first 10 levels (0-9). If you press Ctrl-Shift-i you will then be able to see the digest error.

Any help/suggestions would be great.

mohsen1 commented 7 years ago

I've rewritten thus directive in pure JS. That shouldn't have this problem. I have a plan to use the pure JS implementation here internally as well.

tysenmoore-xse commented 7 years ago

I have modified my branch so you can now run my branch specific tests using the demo/demo.html within a browser (nothing special needed)--in case you are curious what I mean with regard to my specific tests. Just look for the first colored section labelled "Branch Specific Tests".

tysenmoore-xse commented 7 years ago

Is this new "pure JS" directive available for testing? I would be happy to test it.

mohsen1 commented 7 years ago

Here it is: https://github.com/mohsen1/json-formatter-js

Also #35

tysenmoore-xse commented 7 years ago

Thanks for the link. I was able to test the same scenario I previously described using the pure JS version and did not get the digest recursion problem. When are you planning to integrate this into this project/branch?

mohsen1 commented 7 years ago

I'm super busy. PR is welcome

tysenmoore-xse commented 7 years ago

I started an implementation in a branch within my fork--this still needs some work. I've got the concept working in my own project--same implementation. I really need to spend more time getting the npm test to pass, etc. Like I said, it is very close.