11ty / eleventy

A simpler site generator. Transforms a directory of templates (of varying types) into HTML.
https://www.11ty.dev/
MIT License
16.8k stars 485 forks source link

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

Closed zachleat closed 1 month ago

zachleat commented 1 year ago

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 commented 1 year ago

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:

https://github.com/11ty/eleventy/blob/0f44f756a30247515033945e74e171a293bc75d5/src/Engines/Liquid.js#L24

https://github.com/11ty/eleventy/blob/0f44f756a30247515033945e74e171a293bc75d5/src/Engines/Liquid.js#L100

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 commented 1 year ago

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

zachleat commented 1 month ago

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 https://github.com/harttle/liquidjs/discussions/724

zachleat commented 1 month ago

Related: https://github.com/harttle/liquidjs/discussions/724