Add section schema and translation file completion and hover #298
Conversation
packages/theme-check-docs-updater/src/themeLiquidDocsManager.ts
Outdated
Show resolved
Hide resolved
charlespwd
force-pushed
the
feature/section-schema-icc
branch
from
5f681bb to
32d21b9
Compare
charlespwd
force-pushed
the
feature/section-schema-icc
branch
from
d19ccb5 to
465886b
Compare
charlespwd
changed the title
packages/theme-check-docs-updater/src/themeLiquidDocsManager.ts
Outdated
Show resolved
Hide resolved
charlespwd
force-pushed
the
feature/section-schema-icc
branch
7 times, most recently
from
4d4e30e to
4299f84
Compare
| ): Promise<B> { | ||
| for (const loader of dataLoaders) { | ||
| try { | ||
| return transform(await loader()); |
There was a problem hiding this comment.
This exists so that if either transform or the loader fails, we catch and try the next one.
This is to prevent having a loader that resolves to something that doesn't compile, or a loader to something that doesn't parse, etc.
It's to get a guarantee of something that works E2E.
There was a problem hiding this comment.
Awesome stuff! 🔥 I wonder if it would be a good idea to move this logic to the setup. Thus, if the outdated file is valid and the new one is unparseable, we could avoid memoizing the unparseable file. What do you think?
There was a problem hiding this comment.
Hmmmmmmm... I like the idea. We could do this at the downloadFile level with a validator instead.
There was a problem hiding this comment.
I think it would make things a tad complicated. Because setup is stateful and assumes you're overriding a file (latest.json) and compares the results. If we wanted to do this, we'd need to integrate that transform/validity checker in download. It feels like that's going to be complicated.
The fallback should solve the problem of having an unparseable cached file.
| case TranslationFileURI: | ||
| return this.jsonValidationSet.translationSchema(); | ||
| default: | ||
| throw new Error(`No schema for ${uri}`); |
There was a problem hiding this comment.
This branch only happens if someone manually puts a "$schema" in their JSON objects. Which I don't believe we do have in themes.
There was a problem hiding this comment.
Thank you, @charlespwd! Excellent/elegant stuff as always 🔥🔥🚀
I've only left some minor/non-blocking comments. The most significant one is about formatting. If you agree with that, we might handle it in a different PR :)
| ): Promise<B> { | ||
| for (const loader of dataLoaders) { | ||
| try { | ||
| return transform(await loader()); |
There was a problem hiding this comment.
Awesome stuff! 🔥 I wonder if it would be a good idea to move this logic to the setup. Thus, if the outdated file is valid and the new one is unparseable, we could avoid memoizing the unparseable file. What do you think?
| (await jsonLanguageService.completions(params)) ?? | ||
| (await completionsProvider.completions(params)) |
There was a problem hiding this comment.
Personally, I believe we could rely on AugmentedSourceCode (document) to route the language server implementation, thus the Liquid language server doesn't need to wait/depend on the JSON language server (which is probably blazingly fast).
There was a problem hiding this comment.
I'm kind of hesitant here because JSON exists in Liquid files (in {% schema %} tags). So we kind of have to map between sections/*.liquid to the schema of section files from the JSONLanguageService.
Adding another layer of routing would result in a duplicated routing layer I think.
We could make those separate {completions,hover}/providers, but then we'd have to pass the service as a constructor argument and that, too, seems like unnecessary indirection.
packages/theme-language-server-common/src/json/JSONLanguageService.ts
Outdated
Show resolved
Hide resolved
charlespwd
force-pushed
the
feature/section-schema-icc
branch
2 times, most recently
from
5e43859 to
f42638c
Compare
charlespwd
force-pushed
the
feature/section-schema-icc
branch
from
f42638c to
976802c
Compare
What are you adding in this PR?
sections/*.liquid{% schema %}JSON schema intelligent code completionsections/*.liquid{% schema %}JSON schema hover supportlocales/*.jsonJSON schema intelligent code completionlocales/*.jsonJSON schema hover supportFixes Shopify/develop-advanced-edits#140
Fixes Shopify/develop-advanced-edits#141
Fixes Shopify/develop-advanced-edits#142
Fixes Shopify/develop-advanced-edits#143
Fixes Shopify/develop-advanced-edits#147
schema.ICC.mp4
And some translation hover polish
polishing-translation-hover.mp4
Before you deploy
changeset