Request for suggestions: LiquidDoc

[karreiro]

Author: Shopify
Expected end date: December 5, 2024

Background

Snippets are the primary method for reusing logic in Liquid. However, their weak interfaces make it easy to overlook dependencies when rendering them.

Is render missing some dependency?

{% render 'article-card',
  article: item,
  show_image: true,
  show_date: section.settings.article_show_date,
  show_author: section.settings.article_show_author,
  media_aspect_ratio: 1,
  lazy_load: lazy_load
%}

It shouldn't be hard to answer this question. Projects such as Dawn have adopted headers as a convention to define such interfaces. This makes it easier to render snippets, as all dependencies are clearly listed, and there is no need to review the entire snippet to check if we're missing a dependency.

{% comment %}
  Renders an article card for a given blog with settings to either show the image or not.

  Accepts:
  - blog: {Object} Blog object
  - article: {Object} Article object
  - media_aspect_ratio: {String} The setting changes the aspect ratio of the article image, if shown
  - media_height: {String} The setting changes the height of the article image. Overrides media_aspect_ratio.
  - show_image: {String} The setting either show the article image or not.
  - show_date: {String} The setting either show the article date or not.
  - show_author: {String} The setting either show the article author or not.
  - show_badge: {String} The setting either show the blog badge or not.
  - lazy_load: {Boolean} Image should be lazy loaded. Default: true (optional)

  Usage:
  {% render 'article-card' blog: blog, article: article, show_image: section.settings.show_image %}
{% endcomment %}

{%- if article and article != empty -%}
  {%- liquid
    assign ratio = 1
    if media_aspect_ratio != null
      assign ratio = media_aspect_ratio
      # ...

While that convention is very helpful, it relies heavily on manual work to keep the list of dependencies updated and to manually check them when rendering.

Proposal

We should officially support these headers in Liquid as documentation, similar to how JSDoc works. This way, tools such as the Liquid language server can support features around these definitions, such as:

• Code completion for snippet parameters (e.g., {% render "icon", █ )
• Code completion for parameters listed in the {% doc %} tag with the proper type inference (thus, for a @param {string} foo, {{ foo |█ }} displays filters for string objects)
• Linting when a {% render ... %} is missing parameters
• Auto-fixing the list of parameters (@param) in the {% doc %} tag (auto-fix means: add missing parameters and remove unused ones)
• Displaying inline documentation when developers hover over {% render ... %} snippets

{% doc %}
  Renders loading-spinner.

  @param {string} foo - some foo
  @param {string} [bar] - optional bar

  @example
  {% render 'loading-spinner', foo: 'foo' %}
  {% render 'loading-spinner', foo: 'foo', bar: 'bar' %}
{% enddoc %}

<div class="{{ bar }} loading__spinner hidden">
  {{ 'loading-spinner-{{ foo }}.svg' | inline_asset_content }}
</div>

The doc tag behaves just like the comment tag but has a special name to be handled differently by Liquid tooling.

Call for suggestions

We welcome any feedback or opinions on this proposal. Please share your thoughts by December 5, 2024. Your input is valuable as we prepare to begin active development on this initiative.

Context

• Investigate bringing back UndefinedObjects for snippets theme-tools#448
• Introduce snippet dependency annotation theme-tools#114

[galenking]

I really like this idea but the syntax feels brittle—if that’s a thing.

Would something like this work?

{% spec 
  title: string, required, "The title to display", example: "Welcome to our store"
  price: number, required, "The price of the product", example: 19.99
  discount: number, optional, "An optional discount percentage", default: 0, example: 10
  show_badge: boolean, optional, "Flag to display a special badge", default: false, example: true

  output formatted_price: string, "Price formatted with currency symbol", example: "$19.99"
%}

[vitali-bc]

wow... really agree with that!!! many of devs are coming from js freontend frameworks, where they use props...
so that's a really cool idea...

[patryk-smc]

+1 for Galen syntax, feels more like TypeScript than JSDoc

[panoply]

Author: Shopify

lol

[panoply]

• Infer section schema types via LiquidDoc panoply/vscode-liquid#166
• https://x.com/niksavvidis/status/1721870369106141365
• https://x.com/niksavvidis/status/1841275024126468249

[kinngh]

I believe LiquidDocs should stick to comments instead of a separate {% spec ... %} / {% doc %} tag.

• Comments are excluded from rendering by default, so all support for documentation and Intellisense / Autocomplete could be shifted in the IDE
• Comments would keep documentation closer to the code it describes instead of a potentially separate file that contains all the spec.
• No need to learn new tag, and for other products that use Liquid for templating (Email, etc), no update on the Liquid version would be required since LiquidDocs would run on the IDE instead of the server

[panoply]

Correct.

[taksh108]

I had the same first thoughts as @kinngh and agree with him. Existing comment tag + letting the dev tools handle this instead of needing of modifying liquid.
That said, definitely a welcome change. Just today I was switching between the snippet file and my code to check the available params, so yes a big pain.

[benjaminsehl]

@panoply — Definitely thanks and credit where it's due, both in putting this on my radar, and in building the experience you did in the Liquify extension! Huge fan of what you've done there.

These ideas are definitely not new, and something the team has been thinking about for a long time: Shopify/theme-tools#114 — and as you'll see with the syntax and differences from the convention in Dawn, we've decided to align with JSDoc so you can keep the same muscle memory everywhere. Very much borrowing from the prior art we've seen elsewhere, but a good note for us in the future to keep acknowledgements in our RFCs.

I will say I did really like you calling it "LiquidDoc" and I specifically wanted to make that the official name after we talked about aligning the syntax with JSDoc. Thank you & Aurélien for that nudge!

---

@kinngh — we originally were thinking about the {% comment %} tag, and will to keep the same benefits. We definitely want to be judicious when we add new tags, but Liquid is meant to be a declaration of what people want, and being very intentional about semantics is important both to the reader and to a lot of things we want to do in the future.

So while there are some short term tradeoffs here in having to learn a new tag, we feel pretty confident in the long term benefits we can create. Definitely appreciate highlighting those points though — will take it all into consideration.

[benjaminsehl]

I really like this idea but the syntax feels brittle—if that’s a thing.

@galenking can you say more about what feels brittle? The syntax we're proposing is meant to match JSDoc: https://jsdoc.app/

Providing explicit tags and symbols allows us to more predictably/reliably show the right thing with Intellisense — and keeping the same notation as JSDoc means devs can use one syntax everywhere, and tools like Copilot should do a better job of figuring out what to do. There's also the opportunity for our own extension to automatically suggest these docs, and help you keep them in sync with your markup.

[panoply]

These ideas are definitely not new

Indeed, it's nice to see things get ironed out. Crazy how in August we began to discuss that logic panoply/vscode-liquid#166 and then in September you legends were on the same path Shopify/theme-tools#114.

Super excited to see what you come out on and how it was spec'd.

[galenking]

I really like this idea but the syntax feels brittle—if that’s a thing.

@galenking can you say more about what feels brittle? The syntax we're proposing is meant to match JSDoc: https://jsdoc.app/

Providing explicit tags and symbols allows us to more predictably/reliably show the right thing with Intellisense — and keeping the same notation as JSDoc means devs can use one syntax everywhere, and tools like Copilot should do a better job of figuring out what to do. There's also the opportunity for our own extension to automatically suggest these docs, and help you keep them in sync with your markup.

@benjaminsehl, good question. I guess to me it feels like it’s based more around a convention than an explicit structure and it might be hard to learn and remember the syntax—or convention—and it would be easy to be sloppy and inconsistent. If the contents of this doc tag are going to be rendered in tooltips or inline docs, it feels like it needs to be more explicit to avoid inadvertent sloppiness. But, honestly, I don’t have experience with JSDoc so perhaps what’s being proposed makes good sense and has precedent 🤷‍♂

[thomaskimura]

I like this approach and generally have been using the same approach as Dawn. The one thing I have noticed is it's often easy for those comment tags to get outdated if changes are made to a snippet without updating the comment.

[0x15f]

I like this approach and generally have been using the same approach as Dawn. The one thing I have noticed is it's often easy for those comment tags to get outdated if changes are made to a snippet without updating the comment.

Have to agree here. A mirror of JSDoc is completely useless when it is not maintained. I am not saying this is not a worthwhile implementation but it will realistically provide zero benefit to the average developer. An alternate tag such as @galenking’s suggestion could provide benefits beyond just props. E.g) When spec tag is used lock the spec/defined props, skip resolving globals, etc—an alternate tag opens the door for more Intelligent caching and rendering.

If you must then add it to the language server. I don’t see how this belongs in the Liquid implementation.

[davidhollenbeckx]

I'm a huge fan of this. since liquid doesn't typically just error out on missing values I can't count how many times I've had to double check required params. comments work fine if you can enforce them, but as mentioned, there is slippage and there's no feedback from comments in the point where you're using the snippet

if a type system is going to be introduced, tbh, I would prefer it to be strict. personally I would rather get an error in my console that says "hey dummy you forgot a required property". maybe a lint process would be fine but the most common mistakes that make it to production are minor changes in one file by someone less familiar with the code base, that then have unexpected impacts on some seemingly unrelated functionality

[charlespwd]

@0x15f

Have to agree here. A mirror of JSDoc is completely useless when it is not maintained. I am not saying this is not a worthwhile implementation but it will realistically provide zero benefit to the average developer. An alternate tag such as @galenking’s suggestion could provide benefits beyond just props. E.g) When spec tag is used lock the spec/defined props, skip resolving globals, etc—an alternate tag opens the door for more Intelligent caching and rendering.

What if we paired the intellisense with the following behaviour?

• Given you're editing a snippet file with a {% doc %} at the top,
  • We'll warn you if you have a @param that you don't use
  • We'll warn you if you are using a global that we don't recognize as global (i.e. it should be a param)
    • Add an autofixer that adds it to the param list in the {% doc %} tag at the top of the file

e.g.
snippets/foo.liquid

{% doc %}
  Do some stuff
  @param unused - this param will be yellow squiggly lined
{% enddoc %}

{% # using the missing variable that isn't a global %}
{{ missing }}

After accepting linting suggestions (can't be autofixed because we can't be sure of your intentions)

{% doc %}
  Do some stuff
  @param missing - TODO
{% enddoc %}

{% # using the missing variable that isn't a global %}
{{ missing }}

[0x15f]

@charlespwd Those are nice enhancements for intellisense/themecheck/language server and could encourage others to keep the tag/docs current. At worst, it would allow for more tooling to be built to detect and prevent those breaking changes.

Will Intellisense be supported for the online theme editor? Historically, I see the 'quick fixes' or changes happening there that lead to snippet tag/docs being out of date or, worse, usage of a snippet being broken in another area.

[kinngh]

+1 on @0x15f 's suggestion - a lot of quick fixes happen directly on Shopify's Editor and if we don't have pretty/linting support, it all becomes a mess again^T