libp2p / observer-toolkit

🐣 [WIP] toolkit for building libp2p introspection widgets + a few useful out-of-the-box widgets
https://libp2p.io
MIT License
2 stars 2 forks source link

Add events widget, using events data, with data managed by useDatastore #46

Closed AlanSl closed 4 years ago

AlanSl commented 4 years ago

image

This PR also propogates the new events data into the app and does a long-overdue refactor of the way that data more generally is propogated in the app:

It also modifies the mock events data to be a bit more interesting and clearer as a demonstration of the widget:

ovhemert commented 4 years ago

The only thing that I've noticed is that the timeline is acting crazy at start again... thought this was already solved in another PR,... could not find the direct cause, but worth investigating.

ovhemert commented 4 years ago

@AlanSl , can you have a look at the event data as json rendering?

AlanSl commented 4 years ago

@ovhemert Mostly fine but I'm not keen on the custom JSON parsing - JSON.parse is highly optimised (native C++ in v8), so I'm not sure there would be better performance this way, and this way could easily trip up (e.g. { characters inside strings). The perf hit we want to avoid is building a DOM tree to represent potentially deep-nested JSON, so I think it's probably fine to parse the JSON to get the top level.

Can you try a variant of this that parses the JSON and sends the key: value pairs it gets to the existing renderers, and check the performance is okay?

AlanSl commented 4 years ago

👍 and perf is totally fine, I tried this with a ridiculously deep-nested json object tree and the try block and on a pretty slow machine the try block consistently took less than 2ms

AlanSl commented 4 years ago

@ovhemert Latest commit makes the visual presentation same as before except for the addition of a raw JSON column (other front-end features will be done separately to this branch, trying to keep this PR to simply adding the widget and data sourcing):

image

Let me know if you've got any comments on these changes, if not I'll merge this and update and merge the other open PR built on it.

ovhemert commented 4 years ago

LGTM 👍🏻