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

piping inside javascript expressions #87

Open
boehs opened this issue Nov 20, 2024 · 8 comments
Open

piping inside javascript expressions #87

boehs opened this issue Nov 20, 2024 · 8 comments

Comments

@boehs
Copy link

boehs commented Nov 20, 2024

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
Copy link
Contributor

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
Copy link
Author

boehs commented Nov 20, 2024

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
Copy link
Author

boehs commented Nov 20, 2024

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
Copy link
Collaborator

oscarotero commented Nov 20, 2024

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
Copy link
Author

boehs commented Nov 20, 2024

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:

  • Does it make sense to remove this responsibility from tag plugins entirely? If vento has a large plugin ecosystem in the future, random plugins missing the pipe ability with little consistency would be annoying.
  • Is there a way you can call the filter inside javascript? like if I had a filter called reverse I could do like {{> it.filters.reverse(arr) instead arr |> reverse I suppose there must be a way based on the way it compiles to javascript, I will look

@oscarotero
Copy link
Collaborator

oscarotero commented Nov 20, 2024

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
Copy link
Author

boehs commented Nov 20, 2024

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
Copy link
Collaborator

Great!
I'm looking forward your PR!!

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

No branches or pull requests

3 participants