keepsuit / php-liquid

PHP implementation of Liquid markup language
MIT License
8 stars 1 forks source link

Dynamic renders #24

Open dotmbf opened 5 months ago

dotmbf commented 5 months ago

What happened?

https://github.com/keepsuit/php-liquid/blob/main/tests/Integration/Tags/RenderTagTest.php#L73C1-L76C4

Is there any reason why there's no dynamic renders available on {% render %}?

Also is the tag {% sections %} coming, or would we have to integrate this ourselves?

Very fond of this library vs kalimatas to be honest. Keep at it!

How to reproduce the bug

Use {% assign var = 'template' %} {% render var %}

Package Version

0.6.3

PHP Version

8.3.7

Which operating systems does with happen with?

Linux

Notes

Sorry for using bug for this, but there's no feature suggestions available on this repo :(.

dotmbf commented 5 months ago

Follow-up: It's technically possible on Shopify, which is why I'm wondering why it wouldn't be on here as well?

cappuc commented 5 months ago

Hi @dotmbf

render was limited to static template name to be able to compile the compile the child template when compiling the parent and not in the render phase. It shouldn't be to difficult to support dynamic templates, I will look at it.

The sections tag is a shopify specific tag used for their themes and not a standard one

dotmbf commented 5 months ago

Sounds great with dynamic rendering, would be cool!

Fair enough with the sections one. Love the progress though!

cappuc commented 5 months ago

Are you sure that it works with the ruby implementation? Looking at the tests it shouldn't work: https://github.com/Shopify/liquid/blob/77bc56a1c28a707c2b222559ffb0b7b1c5588928/test/integration/tags/render_tag_test.rb#L101

dotmbf commented 5 months ago

I know it works on Shopify and I feel like it's a pretty cool feature?

Unless you want to keep it strictly 1:1, that's fair enough, unfortunately :(

dotmbf commented 5 months ago

Following the trace from https://github.com/Shopify/liquid/blob/main/lib/liquid/tags/render.rb#L47 To: https://github.com/Shopify/liquid/blob/main/lib/liquid/expression.rb#L32

It looks like it supports dynamic variables in any tag?

cappuc commented 5 months ago

I know it works on Shopify and I feel like it's a pretty cool feature?

Unless you want to keep it strictly 1:1, that's fair enough, unfortunately :(

I want to try to keep this library 1:1 with the original but maybe this feature can be enabled with an option

Following the trace from https://github.com/Shopify/liquid/blob/main/lib/liquid/tags/render.rb#L47 To: https://github.com/Shopify/liquid/blob/main/lib/liquid/expression.rb#L32

It looks like it supports dynamic variables in any tag?

It is checked in the render method https://github.com/Shopify/liquid/blob/77bc56a1c28a707c2b222559ffb0b7b1c5588928/lib/liquid/tags/render.rb#L67 I moved the check to the parsing so it can be detected earlier.

dotmbf commented 5 months ago

Ah yeah I see I didn't even notice

Well at this point I'm unsure how I would process it at all, so if it's possible for you to add, with a configurable option that would be greatly appreciated.

cappuc commented 5 months ago

I added support for dynamic template in the dynamic-partial branch, if you want to try it.

I will not merge this right now because it requires some breaking changes and I want to change some other things before tagging a new major version

dotmbf commented 5 months ago

Totally fine I'm using it as a submodule anyways 😊

It seems to be working just fine - thanks a lot!