Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat(devtools-core): Render ST's Visualizer to Include Root Field's Allowed Types #23573
feat(devtools-core): Render ST's Visualizer to Include Root Field's Allowed Types #23573
Changes from 1 commit
df3da17
fd50e40
cf42f7a
1e29755
5e96b73
9a6793a
d4c4901
4990a60
363ced9
dbf8f9b
d80f2e2
3159b80
b40de03
13ee23d
a861e7e
0746731
f02d1a3
a77bf59
9132cbb
4e86139
08db65a
e8c06bf
96a015b
18fbeac
3bc38fb
542d26e
279031d
9219ae1
2d305c6
a3978ab
a284f3a
bffadbe
f44bd59
7d389dc
89d0365
035c070
fdfa737
446ba37
aeee9a2
dfc6f00
0f70aae
3fa33c2
809f3fe
472aa64
8a650d1
1aa4c02
c6519df
8fb3ca9
7413066
3ff216d
60ecfd3
67c0065
6a4995f
735f1d8
c366807
701f22b
4b3f178
cfabb66
df84d25
a451b28
59df510
60e7f80
2856505
af9078e
d9ccdc4
4f0c5ed
f4f0496
ea593f4
0cd81ff
96ee13d
3c40de5
32be645
0be4036
4952676
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
since you are including information about what is allowed in a field, I think it would make more sense to loop over the schema here so currently empty fields would be included.
Also I believe this function processes ObjectNodes, so renaming visualizeVerboseNodeFields to visualizeObjectNode and having it take in an SimpleObjectNodeSchema seems like it would make more sense.
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.
Actually I guess you already have visualizeVerboseNodeFields, so I'm not sure why this exists.
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.
Just double-checking: is there an existing "isLeaf" (or something) helper we could use for this check?
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.
Will double check on this. Thanks for the reminder.
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.
There is a treeValue() utiliy function that is consumed internally in the ST package, but I am not sure if I can expose this to outside package.
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.
Note that function doesn't handle Handle nodes.
@CraigMacomber maybe you can advise on the best publicly available pattern for checking if a node is a leaf using the VerboseTree APIs?
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.
Since leaves in VerboseTree are the same as leaves in regular trees. you can do something like:
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.
Not that I expect us to add a new leaf type anytime soon, but it seems like it would be nice to have this sort of check encapsulated in a single place so we don't have to update all of these hardcoded lists if/when we do add a new leaf type. Not for this PR. but would it be worthwhile to add a centralized
isLeaf
function for this?