Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Origin contains CIRCULAR attributes in state #45

Open
p0o opened this issue Jan 8, 2017 · 10 comments
Open

Origin contains CIRCULAR attributes in state #45

p0o opened this issue Jan 8, 2017 · 10 comments

Comments

@p0o
Copy link

p0o commented Jan 8, 2017

I had this weird circular error when I setup a basic implementation of this lib on my codebase:

Uncaught TypeError: Converting circular structure to JSON

Basically circular structure means an object which is containing elements that refer to the same object and these type of objects are not serializable. So after using an Origin for any element I had this error because somewhere in my react setup I had a serialization like this:

JSON.stringify(state)

In my case it leads to crashing redux dev tools and redux dev tools also show the circulation inside state.tooltip[NAME].Origin._hostnode key as below:

screen shot 2017-01-08 at 5 35 09 pm

Any ideas?

@kuy
Copy link
Owner

kuy commented Jan 8, 2017

@p0o I could also find _hostNode: [CIRCULAR] in simple example demo. But I think it's difficult to avoid Uncaught TypeError: Converting circular structure to JSON error by fixing redux-tooltip because the circulated structure is built by React. This library just stores it in the state as origin.

Solution 1: Use Redux DevTools Extension instead of Redux DevTools. This extension resolves the issue you're facing.

Solution 2: Wrap JSON.stringify with try-catch block. You can define your own stringify function with try-catch. For example, Redux DevTools Extension catches an exception and shows [CIRCULAR] in the inspector.

redux-devtools-extension/src/app/api/index.js

function tryCatchStringify(obj) {
  try {
    return JSON.stringify(obj);
  } catch (err) {
    /* eslint-disable no-console */
    if (process.env.NODE_ENV !== 'production') console.log('Failed to stringify', err);
    /* eslint-enable no-console */
    return jsan.stringify(obj, null, null, { circular: '[CIRCULAR]' });
  }
}

screen shot 0029-01-08 at 23 42 21

@zalmoxisus
Copy link

Vanilla Redux DevTools uses JSON.stringify for persisting the state in persistState enhancer and the extension uses it as well when you add debug_session in the url. It was planned to be fixed in reduxjs/redux-devtools#271. However, we'll remove this enhancer from Redux DevTools Extension 3.0 in favour of zalmoxisus/redux-devtools-extension#276, which I recommend to use to persist data.

However, you might want to get your data with circular references back when persisting the state. In this case you should add serializeState parameter as true and you'll have the data instead of [CIRCULAR], though there could be some issues.

@kuy
Copy link
Owner

kuy commented Jan 9, 2017

@zalmoxisus Thanks for helping! Redux DevTools Extension v3 looks smart.

@p0o I assumed you just want to see the state by using JSON.stringify in somewhere of your code. But if you need to store the state, please follow the advice by @zalmoxisus.

@p0o
Copy link
Author

p0o commented Jan 9, 2017

Thank you both for answers.

Right now I'm not persisting the tooltip part of my state so there shouldn't be a problem. The thing is I am not sure if this approach is correct and does storing circular data in state creates any performance problems or not? If you think it's common and we wouldn't have deeply nested circular references that makes the serialization slow, I would stick to the custom JSON.stringfiy approach for now.

@zalmoxisus
Copy link

I guess storing a reference to the React component will not allow to persist the data (I'm referring not only to the Redux DevTools, but also for cases when persisting the state in localStorage or on the server side). Even if there are no circular data the reference will be lost.

However, usually we don't need to persist a tooltip. If so, you could easily remove the component data for JSON.stringify, by adding a custom toJSON method for actions and states, like we do for events here.

@p0o
Copy link
Author

p0o commented Jan 9, 2017

@zalmoxisus this toJSON method looks very neat. Should I handle its execution by myself manually or there is a better way to do so? I believe you're doing it in a specific way in Redux DevTools?

@zalmoxisus
Copy link

@p0o I didn't look into the sources, but I guess those actions and states come from redux-tooltip and that should be included here, unless you reimplement its action creators and reducers.

@zalmoxisus
Copy link

zalmoxisus commented Jan 9, 2017

To clarify, toJSON method of an object is used natively by JSON.stringify. For example, it's how ImmutableJS is specifying how its custom data types will be stringified.

@p0o
Copy link
Author

p0o commented Jan 9, 2017

Awesome @zalmoxisus that would solve my problem 100%.

Mentioning that, @kuy I think it would be a good idea to add this method to actions and reducers of redux-tooltip so no one else faces this error in the future.

@kuy
Copy link
Owner

kuy commented Jan 11, 2017

I didn't know overwriting JSON.stringify by toJSON. Awesome tips! Thanks @zalmoxisus.

@p0o Yeah, I'd like to add toJSON function to our actions only if NODE_ENV=development by default. I assume most people don't have interest in the inside of origin property. For someone who interests in it, I also provide an option to see the detail of origin. Thanks for your proposal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants