-
Notifications
You must be signed in to change notification settings - Fork 42
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
RSTUDIO-539: Better visualization for mixed and dictionaries #1663
Conversation
…studio into gagik/visualize-json-better
… gagik/visualize-json-better
src/utils/json.ts
Outdated
// If it is possible to serialize the object to a simpler structure with toJSON, do it. | ||
const simplifiedObject = | ||
// eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
(object as any).toJSON != null ? (object as any).toJSON() : object; |
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.
this is a bit hacky but helps clean things up... mainly symbols appearing in the output.
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.
I'd be interested to learn what symbols are appearing, we might want to fix that by ensuring these aren't defined as enumerable by the SDK.
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.
If you're interested in a less hack'y way for doing this:
const simplifiedObject = object && typeof object === "object" && "toJSON" in object && typeof object.toJSON === "function";
@@ -1,6 +1,5 @@ | |||
import { display as displayDataCell } from '../ui/RealmBrowser/Content/Table/types/DataCell'; | |||
import { stringify } from 'flatted'; | |||
|
|||
import { InspectOptions, inspect } from 'util'; |
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.
import { InspectOptions, inspect } from 'util'; | |
import { InspectOptions, inspect } from 'node:util'; |
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.
Is there an advantage to this import over the other, is it to make it explicit this is from part of node? We actually also import util
directly in an earlier file (which I chose to leave untouched to avoid unnecessary side effects) so I could also change it there
import util from 'util'; |
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.
Is there an advantage to this import over the other, is it to make it explicit this is from part of node?
Yes. The most obvious is to avoid loading a node_modules/util
which might have been installed by some mis-behaving dependency.
We actually also import
util
directly in an earlier file
It's most likely because it predates the introduction of the node:
prefix for the built-in modules.
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.
It is more human readable.
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.
👍
Closes #1662.
After:
Before:
Data Used