Repository: twigjs/twig.js (js)

Issue:
Bug with context scoping
While working on #839, I discovered a bug in how the `ParseState` context is being passed around that is causing the `with` tag to not work as intended. Unfortunately, the tests do not currently test whether the context within the tags is isolated and does not affect the context outside of them.

This is most likely causing other issues with context not being properly scoped.

I am working on a PR to simplify how the context is handled and make scoping more explicit.

Pull request description:
Fix passing context around
This fixes #840 by allowing the context to be passed when initially creating an instance of `ParseState`. The old way of passing the context when calling `parse` is still supported to prevent BC breaks, but it no longer used internally.

Patch:
diff --git a/src/twig.core.js b/src/twig.core.js
index d42e8c1..7e63efa 100644
--- a/src/twig.core.js
+++ b/src/twig.core.js
@@ -1007,10 +1007,10 @@ module.exports = function (Twig) {
      * @param {Twig.Template} template The template that the tokens being parsed are associated with.
      * @param {Object} blockOverrides Any blocks that should override those defined in the associated template.
      */
-    Twig.ParseState = function (template, blockOverrides) {
+    Twig.ParseState = function (template, blockOverrides, context) {
         this.renderedBlocks = {};
         this.overrideBlocks = blockOverrides === undefined ? {} : blockOverrides;
-        this.context = {};
+        this.context = context === undefined ? {} : context;
         this.macros = {};
         this.nestingStack = [];
         this.template = template;
@@ -1325,9 +1325,9 @@ module.exports = function (Twig) {
         params = params || {};
 
         return Twig.async.potentiallyAsync(template, allowAsync, () => {
-            const state = new Twig.ParseState(template, params.blocks);
+            const state = new Twig.ParseState(template, params.blocks, context);
 
-            return state.parseAsync(template.tokens, context)
+            return state.parseAsync(template.tokens)
                 .then(output => {
                     let parentTemplate;
                     let url;
diff --git a/src/twig.logic.js b/src/twig.logic.js
index 6f95fc3..a95349a 100644
--- a/src/twig.logic.js
+++ b/src/twig.logic.js
@@ -1339,9 +1339,11 @@ module.exports = function (Twig) {
                         });
                 }
 
+                const isolatedState = new Twig.ParseState(state.template, undefined, innerContext);
+
                 return promise
                     .then(() => {
-                        return state.parseAsync(token.output, innerContext);
+                        return isolatedState.parseAsync(token.output);
                     })
                     .then(output => {
                         return {


Test patch:
diff --git a/test/test.tags.js b/test/test.tags.js
index 4481bd9..34fe154 100644
--- a/test/test.tags.js
+++ b/test/test.tags.js
@@ -26,20 +26,14 @@ describe('Twig.js Tags ->', function () {
         function () {
             it('should support providing context', function () {
                 twig({
-                    autoescape: true,
-                    data: '{% set prefix = "Hello" %}{% with { name: "world" } %}{{prefix}} {{name}}{% endwith %}'
-                }).render().should.equal(
-                    'Hello world'
-                );
+                    'data': '{% with { name: "world" } %}{{ name }}{% endwith %}'
+                }).render().should.equal('world');
             });
 
             it('should support exclusive context', function () {
                 twig({
-                    autoescape: true,
-                    data: '{% set prefix = "Hello" %}{% with { name: "world" } only %}{{prefix}} {{name}}{% endwith %}'
-                }).render().should.equal(
-                    ' world'
-                );
+                    'data': '{% with { name: "world" } only %}{{ name }}{% endwith %}'
+                }).render().should.equal('world');
             });
 
             it('should support not providing context', function () {
@@ -47,6 +41,34 @@ describe('Twig.js Tags ->', function () {
                     'data': '{% with %}{% set foo = 42 %}{{ foo }}{% endwith %}',
                 }).render().should.equal('42');
             });
+
+            it('should handle outer context properly', function () {
+                twig({
+                    'data': '{% set foo = "bar" %}{% with { name: "world" } %}{{ name }} - {{ foo | default("foo is not defined here") }}{% endwith %}'
+                }).render().should.equal('world - bar');
+
+                twig({
+                    'data': '{% set foo = "bar" %}{% with { name: "world" } only %}{{ name }} - {{ foo | default("foo is not defined here") }}{% endwith %}'
+                }).render().should.equal('world - foo is not defined here');
+
+                twig({
+                    'data': '{% set bar = "baz" %}{% with %}{% set foo = 42 %}{{ foo }} - {{ bar | default("bar is not defined here") }}{% endwith %}',
+                }).render().should.equal('42 - baz');
+            });
+
+            it('should scope any context changes within the tags', function () {
+                twig({
+                    'data': '{% set foo = "bar" %}{% with { name: "world" } %}{{ name }} - {{ foo | default("foo is not defined here") }}{% endwith %} - {{ name | default("name is not defined here") }}'
+                }).render().should.equal('world - bar - name is not defined here');
+
+                twig({
+                    'data': '{% set foo = "bar" %}{% with { name: "world" } only %}{{ name }} - {{ foo | default("foo is not defined here") }}{% endwith %} - {{ name | default("name is not defined here") }}'
+                }).render().should.equal('world - foo is not defined here - name is not defined here');
+
+                twig({
+                    'data': '{% set bar = "baz" %}{% with %}{% set foo = 42 %}{{ foo }} - {{ bar | default("bar is not defined here") }}{% endwith %} - {{ foo | default("foo is not defined here") }}',
+                }).render().should.equal('42 - baz - foo is not defined here');
+            });
         }
     );