discoveryjs / cpupro

Rethinking of CPU profile analysis
https://discoveryjs.github.io/cpupro/
MIT License
450 stars 7 forks source link

feat: support custom handler for "open in editor" #8

Open antfu opened 4 months ago

antfu commented 4 months ago

Thank you for this awesome project! It's really helpful!

The only thing I found a bit tedious was locating the exact line of the functions that we were inspecting. Having a "open in editor" feature would be greatly helpful to the overall workflow.

While this could be a bit tricky to do due to the the web being by nature sandboxed, and depending on what context it runs, it might have a different approach to link to the local file. To make it easy to extend and suitable for different scenarios, I think providing an injectable "handler" function would be the best fit, while later we could figure out a better default handling.

I built a VS Code extension for open .cpuprofile with cpupro, and I want it to be able to redirect me directly to the code. I used a local patched version of cpupro, and here is what I have:

https://github.com/lahmatiy/cpupro/assets/11247099/950a72cd-7cdd-4600-bbc9-fe697dd7a297

And here is how I am doing it: https://github.com/antfu/vscode-cpupro/blob/798dcc5a6f07128a420a5caeeec9c7322fd9a261/src/index.ts#L54-L63

(off topic, that I also made https://github.com/localfile-link/localfile-link to allow opening editor from any website, via a client app handle the custom protocol, similar to how Zoom link works - if you are interested in it, I'd be happy to discuss about how we could integrate that for the web version)

lahmatiy commented 4 months ago

Hey @antfu, thank you for your contribution!

It's great to see cpupro being integrated into VS Code. I'd like to propose an alternative approach leveraging Discovery.js, on which cpupro is built. Discovery.js includes an Embed API that allows for communication with sandboxed apps. Although I haven't tested this in the VS Code environment, I'm believe that we can address potential issues if any.

The Embed API enables an embedder to specify actions for the app, which can be executed based on user interactions. This setup ensures that the app can offer additional features when an action is defined, without knowing the specifics of the action's implementation and eliminating the need for "magic" URLs. In VS Code, these actions could be executed according to the extension's permissions.

Here's a simple implementation example for an HTML page:

<iframe id="cpupro-iframe" src="file://path/to/cpupro.html"></iframe>
<script type="module">
    import { connectToEmbedApp } from "@discoveryjs/discovery/dist/discovery-embed.js";

    const disconnect = connectToEmbedApp(document.getElementById('cpupro-iframe'), (app) => {
        app.defineAction('openFile', (filename) => {
           // do something with filename
        });
    });
</script>

In the app:

{
    view: 'link',
    when: '#.actions.openFile',
    onClick: '=#.actions.openFile',
    content: 'text:"Open in editor"'
}

The required changes for the loc-badge:

discovery.view.define('loc-badge', {
    view: 'badge',
    className: 'function-loc',
    data: 'function or $ | marker("function").object |? { ..., text: loc }',
+   onClick: '"openFile".actionHandler(`${module.path}${loc}`)',
    whenData: 'text',
    content: 'html:text.split(/:/).join(`<span class="delim">:</span>`)'
}, { tag: false });

However, it appears that the onClick event handler is not currently supported for the badge component, which seems to be an oversight. I plan to address and resolve this in Discovery.js asap. For now, you may use a link as a temporary workaround:

discovery.view.define('loc-badge', {
-   view: 'badge',
+   view: 'link',
-   className: 'function-loc',
+   className: 'view-badge function-loc',
    data: 'function or $ | marker("function").object |? { ..., text: loc }',
+   onClick: '"openFile".actionHandler(`${module.path}${loc}`)',
    whenData: 'text',
    content: 'html:text.split(/:/).join(`<span class="delim">:</span>`)'
}, { tag: false });

To define an action directly in the app (for testing purposes) use discovery.action.define:

discovery.action.define('openFile', filename => alert(filename));
antfu commented 4 months ago

Thanks for the detailed guides, that look great!

While I do not have the page in an iframe, is it possible to get the discovery from an append script added to report.html? Maybe exposing it to window.discovery similar to discoveryLoader?

lahmatiy commented 4 months ago

Maybe exposing it to window.discovery similar to discoveryLoader?

discoveryLoader is necessary for one specific scenario (loading data from the HTML file itself). window.discovery also works only for a limited number of cases. In fact, there can be several Discovery.js apps on the page as widgets; in this case, window.discovery will point to the last instance. Besides, leaking to the global scope isn't a good practice at all.

You can create an <iframe> in your generated code and make a bridge between cpupro in the <iframe> and the VS Code API. It's not necessary to include HTML inline. See this example on how to inject URLs as resources in the VS Code environment.