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

[vscode extension] Profile annotations and webview #697

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

Conversation

madmath
Copy link

@madmath madmath commented Jan 10, 2025

What are you adding in this PR?

Command to call out to shopify theme profile (Shopify/cli#5109) which will

  • Put profile annotations (vscode "decorations") in the relevant files
  • Show a webview with the speedscope profile, similar to if the command was run from shopify theme profile

Screenshot 2025-01-10 at 8 53 46 AM

Here's the error if shopify theme profile fails to execute (for example, CLI version is not up to date or not authenticated)

Screenshot 2025-01-10 at 9 25 20 AM

What's next? Any followup issues?

Very much a v1, I think we can improve a few ways from here

  • Ability to clear the annotations.
  • Better resiliency regarding calling shopify CLI (open to suggestions).
  • Interactivity between the webview profile and the code.
  • Better UX around calling the command.

Before you deploy

Public API changes, new features

  • I included a minor bump changeset
  • My feature is backward compatible

@madmath madmath requested a review from charlespwd January 10, 2025 14:05
@madmath madmath force-pushed the sfr-profiling branch 3 times, most recently from e9b69f7 to 7f842d9 Compare January 10, 2025 16:45
@madmath madmath requested a review from a team as a code owner January 10, 2025 16:45
1. Replace `execSync` with a `promisify(exec)` to prevent extension hang
2. Rename files to fit our conventions
3. Remove the `_` prefix from private variables
4. Move the spec file next to the profiler file
export async function fetchProfileContents(url: string) {
try {
console.log('[Liquid Profiler] Attempting to load preview for URL:', url);
const { stdout: result, stderr } = await exec(`${SHOPIFY_CLI_COMMAND} --url=${url} --json`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should be able to run runCliCommand instead of exec here after this is merged #708

@charlespwd charlespwd marked this pull request as draft January 15, 2025 13:55
@charlespwd
Copy link
Contributor

I'm converting this back to draft because there's these things we need to worry about:

  • Am I profiling the code I'm looking at?
  • How do we deal with VS Code workspaces?

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