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

Warn users if they use {% include %} in Liquid templates (or update docs to deprecate {% include %}) #2411

Open
AleksandrHovhannisyan opened this issue May 27, 2022 · 0 comments

Comments

@AleksandrHovhannisyan
Copy link
Contributor

AleksandrHovhannisyan commented May 27, 2022

Is your feature request related to a problem? Please describe.

I recently ran into the issue I filed here a while back: #2000.

At first I thought maybe Liquidjs had regressed, but then I saw my comment at the end noting that this was only fixed for the {% render %} tag, not for {% include %} (which is now deprecated). See comment here: harttle/liquidjs#404 (comment)

include and layout are legacy tags which does not provide Context separation. In both Shopify/Liquid and LiquidJS, {% assgin %} in {%include%} files assigns to its parent template, thus will not hide include arguments. So it's by design and that's why {%include%} tag is deprecated.

Describe the solution you'd like

If a user's template language is set to Liquid and they use {% include %}, it would be nice if we could log a warning in the server console noting that this tag is deprecated and may not behave as expected. Otherwise, a user's template may fail silently.

I'm not sure if this is actually possible, though, because 11ty doesn't know when you use a particular tag, right? That's just the template language's job.

Describe alternatives you've considered

  1. Documentation updates to prominently deprecate {% include %}: https://www.11ty.dev/docs/languages/liquid/#supported-features
  2. Add a new config option in liquidjs that allows you to alias {% include %} to {% render %} under the hood. And then 11ty could set this to true by default.

Additional context

Would be happy to work on this if you think it's a worthwhile enhancement.

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

No branches or pull requests

1 participant