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

Fix data being passed into recursive vars #2719

Merged
merged 1 commit into from
Oct 22, 2020

Conversation

jasonvarga
Copy link
Member

This fixes an issue that #2610 introduced.
While that PR looked like it was working fine, it was actually causing data to be overridden where it wasn't supposed to.

This PR reverts that PR, and fixes the underlying issue instead.

One of the most common examples of this is a nav where you have alternative titles (here, a nav_title var).

{{ nav }}
  {{ nav_title or title }}
{{ /nav }}

This will work fine until you're actually on a page that has a nav_title. Then the nav_title of the actual page will trickle down into every item of the nav loop.

A workaround for this is to use the scope parameter to say "I want a variable from this iteration without the parent context leaking into it".

{{ nav scope="iteration" }}
  {{ iteration:nav_title or title }}
{{ /nav }}

PR 2610 broke this behavior by merging the context into every iteration. The scoped variable would no longer only have its own values.

The issue that 2610 was trying to solve was the make the top level variables (like current page vars, globals, etc) available inside the recursive children. Instead, the parser will now merge the data appropriately. The structure tag doesn't need to do it.

@jasonvarga jasonvarga merged commit a755f64 into master Oct 22, 2020
@jasonvarga jasonvarga deleted the fix/recursive-parser-data branch October 22, 2020 20:43
@duncanmcclean
Copy link
Member

Ah whoops! Sorry if I caused an issue 😆

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

Successfully merging this pull request may close these issues.

2 participants