Add theme profile command
#5109
Conversation
Coverage report
Show files with reduced coverage 🔻
Test suite run success2000 tests passing in 904 suites. Report generated by 🧪jest coverage report action from 1a56b88 |
| if (import.meta.resolve) { | ||
| return import.meta.resolve('speedscope/dist/release/index.html') | ||
| } else { | ||
| try { | ||
| const speedscopePath = require.resolve('speedscope/package.json') | ||
| const speedscopeDir = speedscopePath.replace('/package.json', '') | ||
| return `file://${speedscopeDir}/dist/release/index.html` | ||
| } catch (error) { | ||
| throw new Error("Can't find Speedscope package") | ||
| } | ||
| } |
There was a problem hiding this comment.
Seems to work both in tests and with dev shopify. Is there some packaging magic that could make this not work?
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Very cool project! Excited about this change.
I've added a couple of suggestions around changeset management, 1 of which is blocking change.
I wasn't able to get my 🎩 working, but I'll chock it up to user error and try again tommorow
Are there any edge cases you're aware of that we may want to take note of in case we need to patch / support them in the future?
Hum... I'm a little worried about that error. Can you try w/ the |
I had tried to 🎩 as well and got the same error. The json output is |
|
@EvilGenius13 what is your store? |
|
`dev shopify theme profile --url https://coded-courses.myshopify.com/` Only opens a browser page for now, no auth.
Since it's used dynamically
And use import.meta.resolve to load Speedscope
And strengthen how we resolve Speedscope
macournoyer
force-pushed
the
theme-profile
branch
from
8fa9f2f to
a0955f7
Compare
Co-authored-by: James Meng <[email protected]>
Co-authored-by: James Meng <[email protected]>
|
Everything should be working now! We're set to replace https://shopify.dev/docs/storefronts/themes/tools/theme-inspector w/ this! |
| env: 'SHOPIFY_FLAG_THEME_ID', | ||
| }), | ||
| url: Flags.string({ | ||
| description: 'The url to be used as context', |
There was a problem hiding this comment.
Minor question, when we have the default: / does that mean if you were to pass the --url flag without inputting anything after it, that it would default to using /? I tried that out but it didn't work and was curious if it had another meaning.
WHY are these changes introduced?
https://hackdays.shopify.io/projects/19234
We want to expose Liquid profiling via:
The new
theme profilecommand will be called for both. The VSCode extensions will use the--jsonflag.WHAT is this pull request doing?
Add a
shopify theme profile --urlcommand that calls SFR asking for profiling data (using theAcceptheader).Left to do
--jsonarg.theme devcommand.How to test your changes?
Run
dev shopify theme profile --store [YOUR STORE DOMAIN] --url /. Should open a browser window with Speedscope in it.Post-release steps
Measuring impact
How do we know this change was effective? Please choose one:
Checklist