-
Notifications
You must be signed in to change notification settings - Fork 129
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
[Themes] Verify existence of required theme files before initializing storefront session #4810
[Themes] Verify existence of required theme files before initializing storefront session #4810
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @jamesmengo and the rest of your teammates on Graphite |
Coverage report
Test suite run success1890 tests passing in 865 suites. Report generated by 🧪jest coverage report action from 2e91ff9 |
This comment has been minimized.
This comment has been minimized.
/snapit |
🫰✨ Thanks @jamesmengo! Your snapshot has been published to npm. Test the snapshot by intalling your package globally: pnpm i -g @shopify/cli@0.0.0-snapshot-20241107193203
|
ffd46f3
to
7dd5b3f
Compare
pivot: only show this when essential is missing to better determine the cause |
packages/theme/src/cli/utilities/theme-environment/dev-server-session.ts
Outdated
Show resolved
Hide resolved
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.
Looks good, hoping we can get more insights about the missing cookies with this enhanced error 🙌
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.
Thank you, @jamesmengo! I didn't have the chance to test this one, but these changes LGTM :)
WHY are these changes introduced?
Adds validation to ensure required theme files exist before initializing the dev server session. This helps catch issues with incomplete or corrupted themes that could cause the dev server to fail when initializing the preview session.
WHAT is this pull request doing?
verifyRequiredFilesExist
function that checks for essential theme files (layout/theme.liquid
andconfig/settings_schema.json
)How to test your changes?
layout/theme.liquid
orconfig/settings_schema.json
on your shop (more instructions below if needed)theme dev
with the theme flag, and specify the--theme
flag to point at the theme created above. Verify that it fails with appropriate error messageCreating Theme with Missing Assets
p build
shopify-dev theme delete -d
shopify-dev theme dev
The above command should create a new development theme with missing files - you should then see the error being rendered.
Measuring impact
Checklist