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 issue with loop variable access inside nested foreach statements. #135

Conversation

agheorghe-careplanner
Copy link

Accessing and updating loop variables in a nested foreach does not update the value outside.

Here are 2 scenarios I came across where this was happening:

it('nested foreach subprop', function() {
    var tpl = `
      #set($list = [{"prop": "a"}])
      #set($list2 = ["a", "b", "c"])
      #foreach($i in $list)
          #set($fc = $velocityCount - 1)
          #foreach($j in $list2)
              #set($i.prop = "$i.prop$j")
          #end
      #end
      $list
    `
    var ret = render(tpl).trim()
    assert.equal('[{prop=aabc}]', ret);
  })

  it('nested foreach set', function() {
    var tpl = `
      #set($obj = [{
        "SubProp": [
          { "SubSubProp": "a" }
        ]
      }])
      #set($subSubPropRealValue = "b")
      #foreach($sub in $obj)
          #set($fc = $velocityCount - 1)
          #foreach($subsub in $sub.SubProp)
              #set($fcc = $velocityCount - 1)
              #set($sub.SubProp[$fcc].SubSubProp = $subSubPropRealValue)
          #end
          #set($obj[$fc] = $sub)
      #end
      $obj
    `
    var ret = render(tpl).trim()
    assert.equal('[{SubProp=[{SubSubProp=b}]}]', ret);
  })

This seems to be because the foreach loop variables are only stored in the local store and not the global context so the nested foreach does not have access to them.

I'm not sure the following solution is the correct one, but updating the src/compile/set.js:getContext method to merge the local context and the global context seems to fix this issue by making the loop variable available in the global context for the next statements to use.

Thank you for this amazing package, it's been super useful to our team!

@coveralls
Copy link

Coverage Status

Coverage remained the same at 98.602% when pulling ad712d6 on agheorghe-careplanner:fix/nested-foreach-loop-var into f32057a on shepherdwind:master.

Copy link
Owner

@shepherdwind shepherdwind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for you tests code, but the code logic is not great for this condition. I will fix this problem soom

@shepherdwind
Copy link
Owner

2.0.3 published

@agheorghe-careplanner
Copy link
Author

Thanks for you tests code, but the code logic is not great for this condition.

I suspected as much, my knowledge of the package isn't good. Thank you for fixing the 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.

None yet

3 participants