Skip to content
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

Display tracer argument values via DevTool's Reps components #5394

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ochameau
Copy link

@ochameau ochameau commented Mar 6, 2025

This is based on top of #5363 and start using DevTools Reps React component to render nice previews for the recorded objects.

For now, this is a gross integration crash landing Reps into the profiler, but I should probably spawn an npm package for Reps in order to improve the integration into the profiler frontend.

A few takeaways:

  • At first sight, it looks like the difference of React version between profiler and devtools doesn't introduce immediate failure

  • DevTools is using .mjs file extension whereas the profiler is using .js.
    This difference is going to be invisible once Reps is packaged into an npm package.

  • DevTools uses URIs for imports, whereas the profiler uses require path.
    This will still be something to handle while exporting Reps from mozilla-central/DevTools to npm.
    This may be addressed by moving to relative paths.

  • Circular dependencies are forbidden by the profiler build toolchain.
    This is something we can fix in DevTools. There circular dependencies can be avoided.

  • Reps depends on DevTools variables.css in order to render correctly, with the proper colors.
    I'm not sure we want to import so many rules. It also depends on the very devtools specific .theme-light versus .theme-dark class name.
    We may have to manually translate Reps's variable into colors already used in profiler?

  • DevTools isn't using JSX and requires React DOM Factories module/package.
    Sounds fine importing this package as a profiler dependency?
    I imagine this will be transparent once Reps is bundled into an npm package?

  • DevTools uses PropTypes
    Same resolution as React DOM Factories.

@mstange
Copy link
Contributor

mstange commented Mar 6, 2025

Could you add a deploy preview link with an example profile?

@ochameau
Copy link
Author

ochameau commented Mar 6, 2025

Could you add a deploy preview link with an example profile?

Oh, I forgot to cross link the related bugzilla entry (with some more info):
https://bugzilla.mozilla.org/show_bug.cgi?id=1933787#c6

Here is a profile example:
https://deploy-preview-5394--perf-html.netlify.app/public/rxabzrr1e4fcb8f05aavnbm7vsgcp4cn2hg7mtr/stack-chart/?globalTrackOrder=a0w9&hiddenGlobalTracks=1w8&hiddenLocalTracksByPid=4612-0w4~4636-0~4617-0&implementation=js&search=activity-s&thread=f&timelineType=category&v=10

(Note that this is a WIP, I only spent 2hours integrating Reps)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants