Better building blocks for dealing with {% schema %} content
#599
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
- Batteries are included in `@shopify/theme-check-node` - Language server stores the information in the document manager and injects it
JSON Schema conversions of the theme-liquid-docs JSON schemas into TypeScript interfaces
charlespwd
force-pushed
the
fix/572-schema-registries
branch
from
0802d09 to
cbd3872
Compare
charlespwd
commented
Nov 18, 2024
Comment on lines
-39
to
-48
| const promises = visit<SourceCodeType.JSON, Promise<void>>(jsonSchemaAst as JSONNode, { | ||
| async Property(nameNode, ancestors) { | ||
| if ( | ||
| nameNode.key.value === 'name' && | ||
| ancestors.length === ROOT_NODE_ANCESTORS_COUNT && | ||
| isLiteralNode(nameNode.value) | ||
| ) { | ||
| const name = getLiteralValue(nameNode.value); | ||
| const startIndex = node.blockStartPosition.end + getLiteralLocStart(nameNode.value); | ||
| const endIndex = node.blockStartPosition.end + getLiteralLocEnd(nameNode.value); |
There was a problem hiding this comment.
IMO this was a bit too hard just to get the "name" property of the parsed schema.
charlespwd
commented
Nov 18, 2024
| const name = validSchema.name; | ||
| if (!name) return; |
There was a problem hiding this comment.
Node that validSchema.name is autocompleted here:
Something that wasn't true with the previous method.
charlespwd
commented
Nov 18, 2024
Comment on lines
+322
to
+342
| /** | ||
| * Asynchronously get the block schema for 'blocks/${name}.liquid' | ||
| * May return undefined when the theme isn't preloaded. | ||
| * See {@link ThemeBlockSchema} for more information | ||
| */ | ||
| getBlockSchema?: (name: string) => Promise<ThemeBlockSchema | undefined>; | ||
|
|
||
| /** | ||
| * Asynchronously get the section schema for 'section/${name}.liquid' | ||
| * May return undefined when the theme isn't preloaded or if there are none. | ||
| * See {@link SectionSchema} for more information | ||
| */ | ||
| getSectionSchema?: (name: string) => Promise<SectionSchema | undefined>; | ||
|
|
||
| /** | ||
| * (In theme app extension mode) | ||
| * Asynchronously get the block schema for 'blocks/${name}.liquid' | ||
| * May return undefined when the theme isn't preloaded. | ||
| * See {@link AppBlockSchema} for more information | ||
| */ | ||
| getAppBlockSchema?: (name: string) => Promise<AppBlockSchema | undefined>; |
There was a problem hiding this comment.
The important part of the diff. New APIs that are injected into theme-check. These promises can be cached in the language server so that the result is parsed/cached/validated at most once per file version.
The result can be cached between subsequent runs if the file didn't change.
charlespwd
commented
Nov 18, 2024
| export type ValidateJSON<T extends SourceCodeType> = ( | ||
| file: SourceCode<T>, | ||
| export type ValidateJSON = ( | ||
| uri: string, |
There was a problem hiding this comment.
Simplified since we only need the URI
charlespwd
commented
Nov 18, 2024
charlespwd
force-pushed
the
fix/572-schema-registries
branch
2 times, most recently
from
89e7f43 to
f244557
Compare
This was referenced Nov 18, 2024
aswamy
approved these changes
Nov 18, 2024
packages/theme-check-common/src/checks/valid-schema-name/index.ts
Outdated
Show resolved
Hide resolved
charlespwd
force-pushed
the
fix/572-schema-registries
branch
from
f244557 to
e7b28a1
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?
getSectionSchemaandgetBlockSchemacontext utils in theme checkvalidSchemaproperty of the returned value (when no errors happened parsing and the schema is validated by our JSON schema) is a type-safe representation of the schema.ValidSchemaNameas a proof of conceptnew.building.blocks.mp4
What's next? Any followup issues?
Before you deploy
I included a minor bump
changesetThe getBlockSchema & getSectionSchema are technically in the public API even though you don't need to provide them in theme-check-node nor to theme-language-server