zalmoxisus / remote-redux-devtools

Redux DevTools remotely.
http://zalmoxisus.github.io/monitoring/
MIT License
1.8k stars 138 forks source link

Issue on device #33

Closed blackxored closed 8 years ago

blackxored commented 8 years ago

This doesn't appear to happen on the simulator, I've got this error when I access (and then close) an specific view within my app: JSON.stringify cannot serialize cyclic structures.

ST: 2016-07-27 01:19:24.812 [error][tid:com.facebook.react.JavaScript] JSON.stringify cannot serialize cyclic structures. 2016-07-27 01:19:24.815 [fatal][tid:com.facebook.react.RCTExceptionsManagerQueue] Unhandled JS Exception: JSON.stringify cannot serialize cyclic structures.

zalmoxisus commented 8 years ago

Thanks for the report. The stringify function comes from jsan. It handles circular references, but unfortunately in some cases they cannot be identified because they are generated while stringifying. So decycled doesn't handle them.

Maybe @kolodny has any clue about this?

Related to https://github.com/gaearon/redux-devtools/pull/271

kolodny commented 8 years ago

decycle walks the object and checks if it's encountered it before. It should properly handle circular references.

For the record circular references will always be caught since this will fall through to "slow" mode, self references (eg var o = [{}]; o[1] = o[0]; won't be caught unless you call stringify with the proper 4th argument: https://github.com/kolodny/jsan#notes

zalmoxisus commented 8 years ago

@kolodny, thanks for the details. The case described in this issue also fails with jsan.stringify(obj, null, null, false), it throws here. Due to some custom types, some object's parts are generated while stringifying. Is it possible to have an universal replacer which would handle them?

kolodny commented 8 years ago

@zalmoxisus I can't seem to reproduce. The only way I can get close is if I return an object from the replacer argument:

var arr = [ 0 ];
jsan.stringify(arr, () => arr, null, false); // throws
jsan.stringify(arr, () => arr, null, true); // also throws

but the obvious solution there would be to jsan.stringify() the return value of the replacer.

Can you provide an minimal case where this is happening?

zalmoxisus commented 8 years ago

@kolodny, here's what I mean: https://esnextb.in/?gist=20efed752d81c1ca9c113d33eb45c1ce

Jsan is not able to look inside the object, before stringifying, to find circular references.

I just gave the example with mobx observables because I had one (of course there's a solution with just calling mobx.toJS before), but @blackxored probably uses some android stuff.

kolodny commented 8 years ago

@blackxored @zalmoxisus I just pushed a branch https://github.com/kolodny/jsan/tree/fix-mobx-example specifically kolodny/jsan@e72f4d6 . Can you try it out with that and see if the problem is fixed? I'll publish it if works

zalmoxisus commented 8 years ago

@kolodny, that's brilliant! Tried also another case reported before, and it also works now without any workarounds! And I see it works without additional parameters, just jsan.strigify(state). Thanks a lot!

It would be great if @blackxored could also give it a try by replacing jsan package.

kolodny commented 8 years ago

Published to 3.1.3

blackxored commented 8 years ago

I'll test during the weekend. Thanks for looking into this!

On Fri, Jul 29, 2016 at 6:09 PM Moshe Kolodny notifications@github.com wrote:

Published to 3.1.3

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/zalmoxisus/remote-redux-devtools/issues/33#issuecomment-236222534, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIIvHOA3xJUCDzTKnSpGHjR9jDeNRF9ks5qaiWqgaJpZM4JVsKN .

Best Regards,

Adrian Perez https://adrianperez.codes