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

Add support for setting & importing variables via Nunjucks syntax into global variables #2302

Open
tlylt opened this issue Apr 29, 2023 · 5 comments
Assignees

Comments

@tlylt
Copy link
Contributor

tlylt commented Apr 29, 2023

Please confirm that you have searched existing issues in the repo

Yes, I have searched the existing issues

Any related issues?

#2283

What is the area that this feature belongs to?

Author Usability

Is your feature request related to a problem? Please describe.

{% set/import/... %} (nunjucks) not supported in _markbind/variables.md:

Describe the solution you'd like

Support {% set/import/... %} (nunjucks) in _markbind/variables.md

Describe alternatives you've considered

No response

Additional context

No response

@jmestxr
Copy link
Contributor

jmestxr commented Aug 13, 2023

Hello, currently I could think of two methods:

Method 1: Prepend variables.md to every content file and render the result using nunjucks::renderString.

Pros:

  • Straightforward to implement
  • All nunjucks tags can be processed
  • Page variables will override global variables

Cons:

  • Prepending to each content file might be inefficient
  • For templates that use file paths (e.g. {% set/import… ), absolute paths would have to be specified as the relative path of the target file would be different for each content file
  • Kind of like a cheap hack as global variables are not really added to global environment but rather processed for each page

Method 2: Create our own parser to parse nunjucks tags in variables.md to collect all variables, then add each variable to global environment using nunjucks::AddGlobal.

Pros:

  • Legit way to add variables to global environment

Cons:

  • We are parsing the tags on our own instead of letting nunjucks do the work
  • Harder to implement

@jmestxr
Copy link
Contributor

jmestxr commented Sep 5, 2023

@tlylt I'd like to work on this issue

@tlylt
Copy link
Contributor Author

tlylt commented Sep 5, 2023

@tlylt I'd like to work on this issue

Hi James, I'm not particularly confident of either of the solutions you suggested. I was thinking it might be possible to simply enable the nunjuncks processing, just like how other regular pages allow for nunjuncks syntax. Something like that.

If you are available I would like you to work on #2364 which is more urgent than this issue 🙏

@jmestxr
Copy link
Contributor

jmestxr commented Sep 6, 2023

I was thinking it might be possible to simply enable the nunjuncks processing, just like how other regular pages allow for nunjuncks syntax. Something like that.

Right, I have been reading up on possible ways to do so but unfortunately I don't think there is currently a way to process templates globally.

The good thing about addGlobal is that we can also use it to add built-in global variables (https://markbind.org/userGuide/reusingContents.html#built-in-global-variables), eliminating the need for userDefinedVariablesMap etc in VariableProcessor. Could potentially reduce a significant amount of code.

If you are available I would like you to work on #2364 which is more urgent than this issue 🙏

Alright, will be working on this 👍

@LamJiuFong
Copy link
Contributor

Hi, I would like to share my findings and experiments here for future developers:

Approach 1:

Use nunjucks::parse to parse variables.md to fetch the variables and their corresponding values, then use nunjucks::addGlobal or variableProcessor::addUserDefinedVariable to make them global

eg.
if a user does {% set random_arr = [1, 2, 3, 4] %} in variables.md
expected behavior: random_arr can be used as an array anywhere in the site, and should support array operations like

{% for i in random_arr %}
{{ i }}
{% endfor %}

Challenges:
Difficult to determine the value and the data type of the variables as defined by the user. nunjucks::parse returns an abstract syntax tree (AST), we need to extract the values of the variables from the AST
on our own.

For example, we have an array {% set random_arr = [1, 2, 3, 4] %} and object {% set obj = {"first": 1, "second": 2} %}
To view the AST, head to https://ogonkov.github.io/nunjucks-ast-explorer/ and insert the two statements above

  1. For iterables like arrays and objects, their values are represented as child nodes in the AST. From the examples above, 1, 2, 3, 4, "first", 1, "second", 2 are individual child nodes in the AST. We need to find a way to recursively explore the nodes to get the full array/object.
  2. The data type of both iterables are object, we need to find a way to figure out their actual data type as some operations are specific to a certain data type eg. we should only allow indexing by keys for objects {{ obj["first"] }}
  3. There are other use cases for {% set %} which are hard for us to handle. Some examples:
    Nested arrays: {% set nested_arr = [1, 2, [3, 4, [5]]] %}
    Multiple assignments: {% set a, b, c = 100 %}
    Using functions: {% set hi = "hi" | upper %}
    Dependent operations:
{% set subtotal = 5 %}    
{% set taxRate = 0.10 %}    
{% set totalTax = subtotal * taxRate %}

With the challenges above, we need to find another way to precisely extract the value and the data type of the variables before passing into nunjucks::addGlobal or variableProcessor::addUserDefinedVariable.

Notes:

  1. Nunjucks does not expose its parse method as part of its public API, we can however still access it through their code directly, but it is generally not a good idea since we are breaking through the abstraction wall

  2. I wanted to try extracting the values of the variables "after they are parsed and before they are rendered". However, I realised that nunjucks::parse is invoked only when the nunjucks::render or nunjucks::renderString is called. This means that there is no way to "steal" the to-be-rendered values

Related code: 1 2

Approach 2:

Prepend the content of variables.md to all files
Two ways to achieve this:

  1. Create new files and use nunjucks::render to render the newly created files. This will slow down the building process of the app significantly, and we need to find a place to store the new files too.
  2. Extract the content of variables.md and other files as strings, concatenate them and use nunjucks::renderString to render the content. This will also slow down the building process (though not as significant as the method above) and we do not need to create new files. This method generally works well but there is a nit that causes it to be abandoned, it is due to the behavior of nunjucks::renderString check here - nunjucks::renderString ignores newline characters

Notes:

  1. For the extract string method, I think we can use the Singleton method - only extract the content of variables.md once and concatenate the singleton string with other strings. By doing this, we do not have to repetitively extract the content of variables.md. The challenge here is how do we know when to re-extract variables.md's content during the re-building process.
  2. We need to find a way to avoid the {{ ... }} in variables.md from being rendered, my implementation was to comment all <variable> out here, but it might be too hacky

Additional Notes:
Based on my research, a lot of people wanted to do the same thing - given a file, extract all variables and their values defined using {% set %}, however no one has come up with a solution yet. All the comments you see online will ask you to create your own parser. Hence, I think this task is not easy and hopefully someone is able to successfully tackle this in the future!

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

Successfully merging a pull request may close this issue.

3 participants