feat: added onTreePopulated prop

[domagojk]

Needed for https://github.com/stoplightio/platform-internal/pull/7139

[mpodlasin]

@domagojk I've looked at this and linked PR but it's a bit hard for me to understand how necessary is this change.

Can you maybe elaborate a bit more what are you trying to achieve here? 🤔

It worries me, because those changes expose a lot of internals. I am sure it's warranted but maybe we can come up with a way to minimise the amount of added props. 🤷

[domagojk]

@mpodlasin This mirrors what has been done before which is that for each tree node, we are making a node in studio where we control its state (checked and required) so it can be send to the masking api.

I don't see a way of doing that without onTreePopulated because at that instance, tree is ready to be analysed. One small thing that I can do to expose a bit less internals is using (rootNode: RootNode) => void callback signature rather than (tree: JsonSchemaTree) => void.

As for event handlers, I'm using it to calculate the number of nodes in a tree because there is a limit on how big the model can be in order for it to be masked. But I have nothing against in doing that internally, maybe we can return that along with onTreePopulated callback, so it would be (rootNode: RootNode, nodeCount: number) => void and then I can remove treeWalkerEventEmitter property?

[mpodlasin]

@domagojk is there no way to retrieve the number of nodes from JsonSchemaTree directly? 🤔

[mpodlasin]

And yes onTreePopulated is the smaller of my worries, I would prefer to get rid of that event handler somehow :)

[domagojk]

@domagojk is there no way to retrieve the number of nodes from JsonSchemaTree directly? 🤔

As far as i know, there is no way of doing that anymore. tree.count was available in previous JSV version, but it was removed

[mpodlasin]

@domagojk is there no way to retrieve the number of nodes from JsonSchemaTree directly? 🤔

As far as i know, there is no way of doing that anymore. tree.count was available in previous JSV version, but it was removed

I believe this would be much cleaner solution. But I don't want to block you.

Let's make the count a part of onTreePopulated, like you proposed, and we will be okay.

Just for future extensibility I would do something like

onTreePopulated={(tree, { nodeCount }) => ...}

Note how nodeCount is in an object - if in the future you will need something more, we can add it there.

[domagojk]

I think tree.count was removed because tree is now evaluated lazily so we don't know the number of nodes (it will change as you expand the tree). I'm not 100% tho, but I believe this is the case.

Thanks for the tip! 👍 I will try to minimise exposing so many things, but I just found a bug in the implementation so I will have to change this PR probably 🙄

[stoplight-bot]

:tada: This PR is included in version 4.1.0 :tada:

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot :package::rocket: