-
Notifications
You must be signed in to change notification settings - Fork 30
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
[Theme Checks] Added in a check for theme block settings validation #620
Draft
AribaRajput
wants to merge
4
commits into
main
Choose a base branch
from
ariba/presets-settings
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+1,254
−0
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AribaRajput
force-pushed
the
ariba/presets-settings
branch
17 times, most recently
from
December 2, 2024 18:30
b95fc84
to
866ea6c
Compare
charlespwd
reviewed
Dec 3, 2024
charlespwd
reviewed
Dec 3, 2024
charlespwd
reviewed
Dec 3, 2024
packages/theme-check-common/src/checks/valid-block-preset-settings/index.ts
Outdated
Show resolved
Hide resolved
charlespwd
reviewed
Dec 3, 2024
packages/theme-check-common/src/checks/valid-block-preset-settings/index.ts
Outdated
Show resolved
Hide resolved
charlespwd
reviewed
Dec 3, 2024
packages/theme-check-common/src/checks/valid-block-preset-settings/index.ts
Outdated
Show resolved
Hide resolved
charlespwd
reviewed
Dec 3, 2024
packages/theme-check-common/src/checks/valid-block-preset-settings/index.ts
Outdated
Show resolved
Hide resolved
charlespwd
requested changes
Dec 3, 2024
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.
Position information can't be ignored for this check. We'll probably need to rewrite it and iterate over each blocks separately rather than push all of them in the same bucket.
AribaRajput
force-pushed
the
ariba/presets-settings
branch
2 times, most recently
from
December 4, 2024 15:06
b20d7f0
to
0a7932b
Compare
AribaRajput
force-pushed
the
ariba/presets-settings
branch
from
December 5, 2024 21:25
05d6e6c
to
07c3295
Compare
AribaRajput
force-pushed
the
ariba/presets-settings
branch
4 times, most recently
from
December 19, 2024 21:01
3ef48eb
to
eee6ce7
Compare
Finished tests Added changeset
AribaRajput
force-pushed
the
ariba/presets-settings
branch
from
December 19, 2024 22:18
eee6ce7
to
6f90bcc
Compare
AribaRajput
force-pushed
the
ariba/presets-settings
branch
from
December 23, 2024 20:27
8f6b402
to
8745150
Compare
AribaRajput
changed the title
Added in a check for theme block settings validation
[Theme Checks] Added in a check for theme block settings validation
Jan 2, 2025
AribaRajput
force-pushed
the
ariba/presets-settings
branch
from
January 9, 2025 18:28
8745150
to
a262ff6
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What are you adding in this PR?
First part of #602
Adding in a theme check for checking theme blocks for valid preset setting keys. The preset setting keys (and the presets of any theme block preset settings) must match the settings keys configured in either the preset settings or the nested theme block settings.
Docs PR: https://github.com/Shopify/shopify-dev/pull/51243
What's next? Any followup issues?
The next issue is to add the exact same type of theme check for section files, just with more coverage.
What did you learn?
A lot about presets, sections and theme blocks and how they are configured. That was the trickiest part of all of this (managing the types, knowing how to grab nested information, etc).
Before you deploy
changeset
allChecks
array insrc/checks/index.ts
yarn build
and committed the updated configuration filestheme-app-extension.yml
configchangeset
changeset