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

Change liquidjs parameter parsing to use Liquid’s Tokenizer #2679

Closed
zachleat opened this issue Dec 6, 2022 · 6 comments
Closed

Change liquidjs parameter parsing to use Liquid’s Tokenizer #2679

zachleat opened this issue Dec 6, 2022 · 6 comments

Comments

@zachleat
Copy link
Member

zachleat commented Dec 6, 2022

https://liquidjs.com/tutorials/parse-parameters.html#Parse-Parameters-as-Values
https://liquidjs.com/tutorials/parse-parameters.html#Parse-Key-Value-Pairs-as-Named-Parameters

Currently implementation predated those features and is using a one-off parser using moo: https://github.com/11ty/eleventy/blob/0f44f756a30247515033945e74e171a293bc75d5/src/Engines/Liquid.js

I think due to the size of this change, we’ll probably want a configuration escape hatch for folks to swap back to the old parameter parser

Excellent prior art #1263 #1733

@AleksandrHovhannisyan
Copy link
Contributor

Hey, just checking to see if you're accepting help/contributions for this issue. I noticed that 11ty's Liquid engine is currently only using moo in two places:

this.argLexer = moo.compile(Liquid.argumentLexerOptions);

lexer = moo.compile(Liquid.argumentLexerOptions);

By size of the change, do you mean that it's high impact, or are there other changes that would need to be made as well?

@jgerigmeyer
Copy link

@zachleat Any thoughts on @AleksandrHovhannisyan's question above? Would you be interested in contributions for this issue? Thanks!

@zachleat
Copy link
Member Author

zachleat commented Jul 21, 2024

Starting in Eleventy v3.0.0-alpha.18 and newer you can use eleventyConfig.setLiquidParameterParsing("builtin"); to use the parameter parser provided by Liquid.js. Notably this is an opt-in feature and is not enabled by default in 3.0.

The biggest difference y’all will see with this swap is that with Liquid’s provided tokenizer commas are not allowed to separate parameters. In the one-off parameter parser Eleventy supplied, commas were optional. This commas-optional feature is advertised prominently in the docs: https://www.11ty.dev/docs/shortcodes/ Commas are now optional thanks to harttle/liquidjs#724

@zachleat
Copy link
Member Author

Related: harttle/liquidjs#724

nex3 added a commit to nex3/eleventy that referenced this issue Sep 15, 2024
This is an adaptation of 11ty#1733 to use the built-in Liquid argument
parser that was added in 11ty#2679.

Co-Authored-By: Dylan Awalt-Conley <web@dylan.ac>
nex3 added a commit to nex3/eleventy that referenced this issue Sep 20, 2024
This is an adaptation of 11ty#1733 to use the built-in Liquid argument
parser that was added in 11ty#2679.

Co-Authored-By: Dylan Awalt-Conley <web@dylan.ac>
zachleat added a commit to 11ty/11ty-website that referenced this issue Sep 26, 2024
@zachleat
Copy link
Member Author

@zachleat zachleat removed the needs-documentation Documentation for this issue/feature is pending! label Sep 26, 2024
@marioortizmanero
Copy link

marioortizmanero commented Dec 18, 2024

Could the documentation include an example of a Liquid shortcode that uses named parameters? I can't get it to work with:

eleventyConfig.setLiquidParameterParsing("builtin"); // in the config

eleventyConfig.addShortcode("example", function (name, age) {
  return `<p>Name: ${name}, Age: ${age}</p>`;
}); // in the config, later

{% example name: "abc", age: 3 %} // in the markdown file

and I wonder if I'm doing something wrong. Particularly, I'm not sure what the function definition of the shortcode should look like: multiple parameters, a dictionary, something else? The parameters name and age are always undefined. Alternatively, this might be a bug in the implementation?

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

4 participants