Skip to content
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

WIP - Support {% render obj %} #1507

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

WIP - Support {% render obj %} #1507

wants to merge 1 commit into from

Conversation

catlee
Copy link
Contributor

@catlee catlee commented Jan 18, 2022

We would like to add support to the render tag so that it can render externally specified blocks by passing in the block object to the render tag.

For example, in a section rendering a set of blocks:

{% for block in section.blocks %}
  {% case block.type %}
  {% when "title" %}
    {{ block.settings.title }}
  {% when "@app" %}
    {% render block %}
  {% endcase %}
{% endfor %}

We've found this useful in Shopify to render 3rd party extensions contained in "app blocks": https://shopify.dev/themes/architecture/sections/section-schema#render-app-blocks

@sebastienros
Copy link

Wouldn't it make more sense by using {{ obj }} instead? The render tag was introduced also for its caching semantics which I think this PR is breaking.

@catlee
Copy link
Contributor Author

catlee commented Feb 3, 2022

We are currently using {% render block %} in Shopify for rendering app blocks: https://shopify.dev/themes/architecture/sections/section-schema#render-app-blocks

We did also consider using constructs like {{ obj }} or {{ obj.render }} but ultimately decided that {% render obj %} was more appropriate for a few reasons:

  • Using a tag gives us the ability to pass arguments if needed, {% render obj, foo: bar %}
  • We already have {% render 'snippet' %} tag which renders a local template file. Rendering application-defined content using the render tag seems like a natural extension of the behaviour here.

I'm hopeful that this functionality will be generally useful, by allowing application specific drops to render themselves.

@sebastienros
Copy link

We already have {% render 'snippet' %}

And they explicitly made 'snippet' be only string literal in render when it was introduced to "fix" the include tag so the template could be cached.

I still like the concept but I am concerned it won't be accepted in this form. Might even be easier to sell it in the now defunct include tag ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants