ventojs / vento

🌬 A template engine for Deno & Node
https://vento.js.org/
MIT License
212 stars 12 forks source link

piping inside javascript expressions #87

Open boehs opened 5 days ago

boehs commented 5 days ago

Hi, first I wanted to apologise for the flurry of issues, both me and @uncenter are converting large sites to vento out of similar frustrations with nunjucks to you

this issue regards some unintuitive behaviour. As far as we can tell, "javascript-like" expressions like if and for break the pipe syntax

{{ for location,posts of collections.nestedTax[value[0]]?.[value[1]]?.reverse() |> groupby("data." + tax)  }}

groupby is a filter, but it is inaccessible here because of the preceding for clause, where groupby is defined as a filter. I hope the code above demonstrates an intuitive usecase for supporting this behaviour.

uncenter commented 5 days ago

As far as I can tell this is the way Vento is designed - expressions are JavaScript only, pipes are a separate Vento concept that only works in specific tags in specific ways. I'll give a quick example anyways:

https://livecodes.io/?x=id/x2dgdsr593y

{{ set abc = "a" |> escape }}
{{ if abc }}abc{{ /if }}

This code runs and results in abc.

Replacing the abc variable in the if tag with the pipe expression used to set it:

https://livecodes.io/?x=id/p7yyyzwhzdv

{{ if "a" |> escape }}abc{{ /if }}

Results in different output (on the playground just the raw input text is shown, so I assume an error is occuring).

boehs commented 5 days ago

Related: the following works:

{{ set c = post.data.description |> renderMd }}
{{ Card({
  class: "p-summary",
  content: c
}) }}

The following does not

{{ Card({
  class: "p-summary",
  content: post.data.description |> renderMd
}) }}
boehs commented 5 days ago

One other: parens break it: {{ set a = ("<a>" |> escape) == "a" }}

We've skimmed the code and believe this issue is a bug report, but are not sure where yet.

oscarotero commented 5 days ago

Vento pipes must be always at the end of the tag. for example, this works: {{ set a = "a" |> toUpperCase }} but this doesn't: {{ set a = ("a" |> toUpperCase) + "b" }}

Maybe, when pipes are implemented natively in JavaScript, this feature can be removed from Vento.

Currently, the if tag doesn't have support for pipes. {{ if " " |> trim }} doesn't work. Maybe is something that could be implemented.

And pipes in the for is applied to the iterable, not to every element. For example: {{ for a of [1, 2, 3].reverse() }} is the same as {{ for a of [1, 2, 3] |> reverse }}. I'm not sure if this is what you're asking for.

boehs commented 5 days ago

Vento pipes must be always at the end of the tag. for example, this works: {{ set a = "a" |> toUpperCase }} but this doesn't: {{ set a = ("a" |> toUpperCase) + "b" }}

Maybe, when pipes are implemented natively in JavaScript, this feature can be removed from Vento.

So I suppose that's why the card example doesn't work as well? Is there any way it could be supported anywhere? I’m especially interested in support for (x |> y) + (x |> y) (allowing it at the end of a statement within parentheses)

Currently, the if tag doesn't have support for pipes. {{ if " " |> trim }} doesn't work. Maybe is something that could be implemented.

Yes, this would be nice. I assume the implementation is just adding a compileFilters in the plugin?

And pipes in the for is applied to the iterable, not to every element. For example: {{ for a of [1, 2, 3].reverse() }} is the same as {{ for a of [1, 2, 3] |> reverse }}. I'm not sure if this is what you're asking for.

Maybe, I was having issues with it, maybe related to the first issue, the at the end part.

A couple other things:

oscarotero commented 5 days ago

Is there any way it could be supported anywhere?

It's hard because it requires to transpile the javascript code (currently, Vento only takes the js code and run it as is). Filters are stored in the __env.filters object (see the code here: https://github.com/ventojs/vento/blob/main/src/environment.ts#L313) Technically, you can simply run {{ __env.filters.reverse(arr) }} instead of {{ arr |> reverse }} (the only difference is the this object).

I assume the implementation is just adding a compileFilters in the plugin?

Yes, take a look to how is implemented in for: https://github.com/ventojs/vento/blob/main/plugins/for.ts#L34 The if is similar, just passing the condition https://github.com/ventojs/vento/blob/main/plugins/if.ts#L23

Does it make sense to remove this responsibility from tag plugins entirely?

It's not possible because tags apply the filters differently. For example for item of iterable, filters are applied to the iterable element. set item = value, filters are applied to the value element, {{ echo |> toUpperCase }} hello world {{ /echo }} the filter is applied to the content between the starting and end tags.

Is there a way you can call the filter inside javascript?

Yes, as said in the first comment, you can run __env.filters.reverse. Perhaps we can create an option to store all filters in a variable to make them more accessible (in the same way as we have the dataVarname option to it.

boehs commented 5 days ago

Yes, take a look to how is implemented in for: https://github.com/ventojs/vento/blob/main/plugins/for.ts#L34 The if is similar, just passing the condition https://github.com/ventojs/vento/blob/main/plugins/if.ts#L23

Yes, as said in the first comment, you can run __env.filters.reverse. Perhaps we can create an option to store all filters in a variable to make them more accessible (in the same way as we have the dataVarname option to it.

I'll do both of these in a couple days if you don't 10x dev me first! First just need to wrap up this monster:

image
oscarotero commented 5 days ago

Great! I'm looking forward your PR!!