-
Notifications
You must be signed in to change notification settings - Fork 25
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
Feature/827 Integrate Event Framework #121
Feature/827 Integrate Event Framework #121
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason why we are not considering something like --> https://www.npmjs.com/package/jspon to handle circular references?
@mdebarros, I prefer to keep things simple and to me the solution from MDN beats all other, when it comes to using JSON.stringify() |
@rmothilal, I'm trying to fix an issue with JSON.stringify() in quoting-service here. |
@mdebarros, if there's anything to consider, it is to ban the usage of JSON.stringify and somehow impose it (in favour of CentralServices.Util.stringify) Also, the alternative for deep cloning - JSON.parse(JSON.stringify(obj)) is still with TODO comments:
|
@ggrg : I assume you have tested this solution on quoting-service right? Also, why has not been an issue previously? |
Yes, I tested it, and also wrote unit tests, which also do. I guess it was not an issue previously, because it was not tested! Checkout master, trigger that branch and you'll see the issue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Happy
also util.inspect removes the circular reference but replaces it with |
Because "The output of util.inspect may change at any time and should not be depended upon programmatically." from here. |
Changes in this PR: