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

Implement EagerFromTag and EagerImportTag #560

Merged
merged 15 commits into from
Dec 18, 2020
Merged

Conversation

jasmith-hs
Copy link
Contributor

Part of #532

This PR finishes the logic for importing with either a from tag or an import tag. Over half the additions here are tests, but the logic is somewhat complex/lengthy so I'll do my best to explain it well.

EagerImportTag

The import tag is mainly used to import macros from another file, but it also supports importing values set from another file. The deferred importing of both macros and values need to be handled. The goal is to make the output from eager execution not depend on any other files. So the data from the imported file needs to be serialised in some way to the output from eager execution if those values are dependent on deferred values.

Import tags can either use an alias or not. A tag that imports with an alias, like {% import x as y %} stores all the macros and values from x like y.z. Without an alias like {% import x %} would store a macro declared in x, foo(), as simply foo() (as opposed to y.foo()). This is how imports work normally so the output when an import gets deferred needs to be able to populate these alias maps that can be multi-leveled (as there can be multiple levels of importing). This is done by tracking the full import resource alias name so that given 3 files "file1.jinja", "file2.jinja", "file3.jinja" as:

{% set bar = 1234 %}
{% import 'file1.jinja' as y %}
{% import 'file2.jinja' as x %}

Would result in a context map that looks like: {'x': {'y': {'bar': 1234}}} with the value of bar accessable with: {{ x.y.bar }}

If instead, bar depended on a deferred value, the simplified output from executing "file3.jinja" would look something like:

{% set y = {} %}
{% set bar = deferred %}
{% do y.update({'bar': bar})
{% set x = {} %}
{% do x.update({'y': y}) %}

This is achieved with a process that I'll briefly describe. When an import tag is executed, I now add an additional meta value on the context: Context.IMPORT_RESOURCE_ALIAS_KEY is the key for the value which marks the full import alias. In the above example, while executing 'file1.jinja', this would be x.y. When executing a set tag, in addition to executing/reconstructing the set tag, a do-update tag is constructed if something depends on a deferred value. When executed during a second pass, this will update the alias map to hold the desired value, while allowing any other tokens that may be deferred in the imported file to be able to execute properly the second time.

After the import file is interpreted, that child context gets integrated with the (parent) context in which the file was imported. If the import was done using an alias, the alias map is copied to the parent context. If it's not done using an alias, then the child bindings are simply copied over. If anything was deferred during the import, then a do-update tag is created that defines the alias map (which is essentially a copy of the child context's bindings) that can be loaded during a second pass.

Also, there are macros that get imported. Without an alias, they will get put as global macros, but with an alias, they will get loaded onto the alias map as what I refer to as "localMacros". They're called with alias.macro(), like in the normal import tag functionality. See #547 for the deferred macro logic when a macro function also needs to be deferred.

The caveat with this approach is that name clashes from deferred values don't get handled properly in one case:

  1. A variable is set on the context before an import.
  2. A file is imported using an alias.
  3. Within that imported file, a variable is declared using the same name, but it depends on a deferred value so it's setting has to be deferred.
  4. Outside of the imported file, the variable set in step 1 is used, but it is now deferred and will have the value of the variable set in step 3 when run through a second pass.

This is an extremely niche case and I couldn't think of a way to solve it. I highly doubt there are any real-world usages that this would impact.

EagerFromTag

From tags are much easier because you explicitly specify which variables or macros to import from the file. The file which is being imported gets executed without anything really special. From tags also allow aliases for individual variables or macros, but these are different as they are simply renaming the variable or macro. In the case where a value needs to be deferred, then a set tag is created that just sets a variable with "new_name" to the value of "old_name" like: {% set new_name = old_name %}.

There is a similar niche case that exists for this logic as well:

  1. A variable (say x) is set on the context before a from tag
  2. A from tag imports a variable with that same name and calls it something else ({% from file import x as y %})
  3. Within the file, the value of x can not be resolved, and it must be a deferred value
  4. Outside of the imported file, x, but it is now deferred and will have the same value as y when run through a second pass.

cc @jboulter @Joeoh this should be the remaining logic that's needed for eager execution to be usable

@jasmith-hs jasmith-hs mentioned this pull request Dec 7, 2020
32 tasks
Base automatically changed from eager-print-do to master December 9, 2020 22:23
@jasmith-hs
Copy link
Contributor Author

Any questions or concerns before I merge this in? Again, this logic is only impacting Eager Execution, so any changes, including retroactive are easy to make.

@jasmith-hs jasmith-hs requested review from Joeoh and boulter December 17, 2020 17:29
@jasmith-hs jasmith-hs merged commit 93257ff into master Dec 18, 2020
@jasmith-hs jasmith-hs deleted the eager-from-import-a branch December 18, 2020 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants