-
-
Notifications
You must be signed in to change notification settings - Fork 960
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
Add tracing to virtual_dom #1949
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.
I'm torn on testing the traces we output. The tests added in this PR are for APIs that are already public (new and rebuild) but for internal functions it feels like these tests would be very fragile. For example, it isn't defined exactly how many times poll_tasks should be called every rebuild, so we probably shouldn't test how many times that trace is seen
On the other hand, if tracing is the API we plan to use for dev tools in the future, it might make sense to test that tracing is working correctly
The specific tests added in this PR don't seem flakey and are easy enough to remove if they do end up being problematic, so I'm going to go ahead and merge this PR
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 wouldn't recommend testing the traces outputted, but for the purposes of this PR/issue, i felt it was necessary to prove completion of the work.
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.
Thanks for adding tracing to core! This will make it much easier to see what is going on inside of the virtual dom
Address #1161; Add tracing to virtual dom:
example log