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

Feature request: Shopify's additional filters #384

Open
daviburg opened this issue Jun 8, 2020 · 7 comments
Open

Feature request: Shopify's additional filters #384

daviburg opened this issue Jun 8, 2020 · 7 comments
Labels

Comments

@daviburg
Copy link
Member

daviburg commented Jun 8, 2020

Dotliquid version

(any)

Expected behavior

dot Liquid implements Shopify's additional filters documented at:
https://shopify.dev/docs/themes/liquid/reference/filters/additional-filters#json

Actual behavior

dot Liquid implements only the standard filters at:
https://shopify.github.io/liquid/filters/abs/

Steps to reproduce the Problem (you can add files)

TBD: some filters may be of higher value than others, such as the json filter which other implementations of Liquid have also added a custom version of, e.g. Liquid Jekyll where they implemented a custom ‘jsonify’ filter for that purpose

@daviburg
Copy link
Member Author

daviburg commented Jun 8, 2020

Possible source of inspiration in other liquid open source (BSD3) licensed implementation: https://github.com/OrchardCMS/OrchardCore/blob/dev/src/OrchardCore.Modules/OrchardCore.Liquid/Filters/JsonFilter.cs

@daviburg
Copy link
Member Author

daviburg commented Jun 8, 2020

Fluid's implementation of the same using System.Text.Json:
https://github.com/sebastienros/fluid/pull/194/files

@microalps
Copy link
Contributor

Newtonsoft allows us to implement this in a one liner. We use this, but rather not force dependency for this. https://www.newtonsoft.com/json/help/html/Overload_Newtonsoft_Json_JsonConvert_SerializeObject.htm

@daviburg
Copy link
Member Author

daviburg commented Feb 7, 2021

Newtonsoft allows us to implement this in a one liner. We use this, but rather not force dependency for this. https://www.newtonsoft.com/json/help/html/Overload_Newtonsoft_Json_JsonConvert_SerializeObject.htm

Newtonsoft is a popular library. But adding this dependency will very likely not be a welcome change for a number of existing DotLiquid consumers. There would likely we assembly loading conflicts, and some versions of Newtonsoft even had breaking changes.
If we enabled support via Newtonsoft or any other external library, we will need to figure out a scheme to make it opt-in.

@sebastienros
Copy link

Use System.Text.Json and with multi-targeting no need for a new ref for recent TFMs, check Fluid's PR in the comments.

@microalps
Copy link
Contributor

microalps commented Mar 8, 2021

For now my solution is documented on https://github.com/dotliquid/dotliquid/wiki/DotLiquid-Syntax-Compatibility#unsupported-filters. As this is outside of the library no dependency is introduced unless they opt in by coding the filter.

robin-parker pushed a commit to robin-parker/dotliquid that referenced this issue Jul 1, 2021
- String filters:  Md5 Sha1, Sha256, HmacSha1, HmacSha256
- Additional filters (dotliquid#384): Json
@microalps
Copy link
Contributor

In regards to the json filter, more research is needed if Shopify allows jsonification of unsafe objects. Using Json.net or System.Text.Json to serialize will not exclude objects that DotLiquid natively does not output. This included classes not inheriting from drops and are not marked safe, ILiquidizable objects, or objects with specific allowed members in the Liquid attribute

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants