amitnovick / xstate-devtools

MIT License
102 stars 11 forks source link

Errors on circular object structures #10

Open tivac opened 5 years ago

tivac commented 5 years ago

We've got some complicated xstate machines that apparently have circular references in them, and the usage of JSON.stringify is causing the extension to fail to work with our app. It also causes our entire app to fail because the injected code is exploding, so some additional bomb-proofing there might be prudent.

https://github.com/amitnovick/xstate-devtools/blob/68b16bf6fc677d28adb5cdbfffc1122d297b8ddf/public/extension/injected/injected.js#L16

injected.js:19 Uncaught TypeError: Converting circular structure to JSON
    --> starting at object with constructor 'Object'
    |     property 'done.invoke.id' -> object with constructor 'Array'
    |     index 0 -> object with constructor 'Object'
    |     property 'source' -> object with constructor 'Object'
    --- property 'on' closes the circle
    at JSON.stringify (<anonymous>)
    at Object.connect (injected.js:19)
    ...

Here's a sanitized version of the original state that the error is complaining about, I'm not totally sure what the issue is yet.

state : {
    invoke : {
        id  : "id",
        src : () => // promise-returning function,

        onDone : [
            {
                target : "complete",
                cond   : (ctx, { data }) => !data.field,
            },

            {
                target  : "data",
                actions : assign({
                    value : (ctx, { data }) => data.value,
                    connection : (ctx, { data }) => {
                        data.field = data.value;

                        return data;
                    },
                }),
            },
        ],
        onError : die("FAILED"),
    },
},
amitnovick commented 5 years ago

Thanks for reporting this! 🤗 Good point about bomb-proofing, will put some more safety measures in place that should let things like this fail gracefully and not crash the page.

Regarding the error itself, could you try to see how the Visualizer handles this? it would help me to figure out if I perhaps should pull some patches from XState-Viz.

tivac commented 5 years ago

Seems to work fine in the visualizer, though it's a little tough to know for sure since I've had to modify the actual statechart heavily to be able to use it in there.

https://xstate.js.org/viz/?gist=f8ec83acba677f5e88bfeda5e7ab39e3

Our app has some other problems in 4.7.0-rc5 that I haven't chased down yet, but those don't even get a chance to pop atm because of the circular structure issue.

amitnovick commented 5 years ago

Added the bugfix in https://github.com/amitnovick/xstate-devtools/commit/6f9dc9cc3463cf8a4a60ec769b65d69fcbbf4206 👍

Updated the extension to 0.5.3, please give it a try sometime @tivac to make sure it's solved.

tivac commented 4 years ago

Finally circling back around to give this another shot. Good news, it doesn't blow up our app any more!

Bad news, still fails to serialize our statechart to JSON using JSON.stringify() 😞

I'm investigating why and will try to attach a minimal repro, because I desperately want this to work.

amitnovick commented 4 years ago

I'm investigating why and will try to attach a minimal repro

That would be helpful 👍

tivac commented 4 years ago

Looks like either the extension (or xstate@next) doesn't properly JSON-encode nested invoke'd child machines.

https://codesandbox.io/s/xstate-childgrandchild-machines-elgdf

Standalone version to see the warning, https://elgdf.csb.app/

Seems like xstate@next shouldn't include circular references in its .toJSON() for that sort of node? Or maybe the extension could use something like json-stringify-safe?

It also seems like the sheer size of our statechart may be preventing rendering, I commented out all our nested invoke usage and while the error was gone I still never got anything to render in the devtools. If there's no error output but also nothing rendering in the devtools pane is there any way to debug it further?

amitnovick commented 4 years ago

@tivac thanks for bringing this to my attention, I appreciate your effort in digging all this info out to make debugging easier. Also, apologies for the delay in response.

This is not a final solution yet, but just an update that I've checked out your code in the extension and to narrow down the problem, it seems that only the parent machine is not displayed in the list, whereas child and grandchild are displayed.

Also, with a single invoked machine both parent and child machines are visualized, but as soon as the third is added, it is omitted from the list.

I will try to dig in some more later this week to figure out what's going on.

ayush987goyal commented 3 years ago

@amitnovick The same problem seems to exist if any data in the context/event has a circular object structure (for example dom element references).

We have a use-case where we need to add the reference to a canvas element in the state machine context but the extension seems to break since it is not able to stringify these element. These dom elements can't be inherently stringified that easily. Do you know of any workaround for the same?