refactor: src/scope.js

src/scope.js

@@ -1,38 +1,50 @@
+'use strict'
 const _ = require('./util/underscore.js')
 const lexical = require('./lexical.js')
 const assert = require('./util/assert.js')
 
 var Scope = {
   getAll: function () {
-    var ctx = {}
-    for (var i = this.scopes.length - 1; i >= 0; i--) {
-      _.assign(ctx, this.scopes[i])
-    }
-    return ctx
+    return this.scopes.reduce((ctx, val) => Object.assign(ctx, val), Object.create(null))
   },
-  get: function (str) {
-    try {
-      return this.getPropertyByPath(this.scopes, str)
-    } catch (e) {
-      if (!/undefined variable/.test(e.message) || this.opts.strict_variables) {
-        throw e
+  get: function (path) {
+    let paths = this.propertyAccessSeq(path)
+    let scope = this.findScopeFor(paths[0])
+    return paths.reduce((value, key) => this.readProperty(value, key), scope)
+  },
+  set: function (path, v) {
+    let paths = this.propertyAccessSeq(path)
+    let scope = this.findScopeFor(paths[0])
+    paths.some((key, i) => {
+      if (!_.isObject(scope)) {
+        return true
       }
-    }
-  },
-  set: function (k, v) {
-    var scope = this.findScopeFor(k)
-    setPropertyByPath(scope, k, v)
-    return this
+      if (i === paths.length - 1) {
+        scope[key] = v
+        return true
+      }
+      if (undefined === scope[key]) {
+        scope[key] = {}
+      }
+      scope = scope[key]
+    })
   },
   push: function (ctx) {
     assert(ctx, `trying to push ${ctx} into scopes`)
-    return this.scopes.push(ctx)
+    this.scopes.push(ctx)
   },
-  pop: function () {
-    return this.scopes.pop()
+  pop: function (ctx) {
+    if (!arguments.length) {
+      return this.scopes.pop()
+    }
+    let i = this.scopes.findIndex(scope => scope === ctx)
+    if (i === -1) {
+      throw new TypeError('scope not found, cannot pop')
+    }
+    return this.scopes.splice(i, 1)[0]
   },
   findScopeFor: function (key) {
-    var i = this.scopes.length - 1
+    let i = this.scopes.length - 1
     while (i >= 0 && !(key in this.scopes[i])) {
       i--
     }
@@ -41,32 +53,19 @@ var Scope = {
     }
     return this.scopes[i]
   },
-  unshift: function (ctx) {
-    assert(ctx, `trying to push ${ctx} into scopes`)
-    return this.scopes.unshift(ctx)
-  },
-  shift: function () {
-    return this.scopes.shift()
-  },
-
-  getPropertyByPath: function (scopes, path) {
-    var paths = this.propertyAccessSeq(path + '')
-    if (!paths.length) {
-      throw new TypeError('undefined variable: ' + path)
+  readProperty: function (obj, key) {
+    let val
+    if (key === 'size' && (_.isArray(obj) || _.isString(obj))) {
+      val = obj.length
+    } else if (_.isNil(obj)) {
+      val = undefined
+    } else {
+      val = obj[key]
     }
-    var key = paths.shift()
-    var value = getValueFromScopes(key, scopes)
-    if (_.isNil(value)) {
-      throw new TypeError('undefined variable: ' + key)
+    if (_.isNil(val) && this.opts.strict_variables) {
+      throw new TypeError(`undefined variable: ${key}`)
     }
-    while (paths.length) {
-      key = paths.shift()
-      value = getValueFromParent(key, value)
-      if (_.isNil(value)) {
-        throw new TypeError('undefined variable: ' + key)
-      }
-    }
-    return value
+    return val
   },
 
   /*
@@ -78,16 +77,17 @@ var Scope = {
    * accessSeq("foo[bar.coo]")    // ['foo', 'bar'], for bar.coo == 'bar'
    */
   propertyAccessSeq: function (str) {
-    var seq = []
-    var name = ''
-    var j
-    var i = 0
+    str = String(str)
+    let seq = []
+    let name = ''
+    let j
+    let i = 0
     while (i < str.length) {
       switch (str[i]) {
         case '[':
           push()
 
-          var delemiter = str[i + 1]
+          let delemiter = str[i + 1]
           if (/['"]/.test(delemiter)) { // foo["bar"]
             j = str.indexOf(delemiter, i + 2)
             assert(j !== -1, `unbalanced ${delemiter}: ${str}`)
@@ -115,6 +115,10 @@ var Scope = {
       }
     }
     push()
+
+    if (!seq.length) {
+      throw new TypeError(`invalid path:"${str}"`)
+    }
     return seq
 
     function push () {
@@ -124,42 +128,6 @@ var Scope = {
   }
 }
 
-function setPropertyByPath (obj, path, val) {
-  var paths = (path + '').replace(/\[/g, '.').replace(/\]/g, '').split('.')
-  for (var i = 0; i < paths.length; i++) {
-    var key = paths[i]
-    if (!_.isObject(obj)) {
-      // cannot set property of non-object
-      return
-    }
-    // for end point
-    if (i === paths.length - 1) {
-      return (obj[key] = val)
-    }
-    // if path not exist
-    if (undefined === obj[key]) {
-      obj[key] = {}
-    }
-    obj = obj[key]
-  }
-}
-
-function getValueFromParent (key, value) {
-  return (key === 'size' && (_.isArray(value) || _.isString(value)))
-    ? value.length
-    : value[key]
-}
-
-function getValueFromScopes (key, scopes) {
-  for (var i = scopes.length - 1; i > -1; i--) {
-    var scope = scopes[i]
-    if (scope.hasOwnProperty(key)) {
-      return scope[key]
-    }
-  }
-  throw new TypeError('undefined variable: ' + key)
-}
-
 function matchRightBracket (str, begin) {
   var stack = 1 // count of '[' - count of ']'
   for (var i = begin; i < str.length; i++) {

src/util/underscore.js

@@ -57,19 +57,10 @@ function forOwn (object, iteratee) {
 function assign (object) {
   object = isObject(object) ? object : {}
   var srcs = Array.prototype.slice.call(arguments, 1)
-  srcs.forEach(function (src) {
-    _assignBinary(object, src)
-  })
+  srcs.forEach((src) => Object.assign(object, src))
   return object
 }
 
-function _assignBinary (dst, src) {
-  forOwn(src, function (v, k) {
-    dst[k] = v
-  })
-  return dst
-}
-
 function last (arr) {
   return arr[arr.length - 1]
 }

tags/for.js

@@ -84,7 +84,7 @@ module.exports = function (liquid) {
             }
             throw e
           })
-          .then(() => scope.pop())
+          .then(() => scope.pop(context))
       }).catch((e) => {
         if (e instanceof RenderBreakError && e.message === 'break') {
           return

test/scope.js

@@ -59,10 +59,16 @@ describe('scope', function () {
       }
       expect(fn).to.not.throw()
       expect(scope.get('notdefined')).to.equal(undefined)
-      expect(scope.get('')).to.equal(undefined)
       expect(scope.get(false)).to.equal(undefined)
     })
 
+    it('should throw for invalid path', function () {
+      function fn () {
+        scope.get('')
+      }
+      expect(fn).to.throw('invalid path:""')
+    })
+
     it('should throw when [] unbalanced', function () {
       expect(function () {
         scope.get('foo[bar')
@@ -116,10 +122,9 @@ describe('scope', function () {
       expect(scope.get('posts[category.diary[0]].name'), 'A Nice Day')
     })
 
-    it('should create in parent scope if needed', function () {
-      scope.push({})
-      scope.set('bar.coo', 'COO')
-      expect(scope.get('bar.coo')).to.equal('COO')
+    it('should create parent if needed', function () {
+      scope.set('a.b.c.d', 'COO')
+      expect(scope.get('a.b.c.d')).to.equal('COO')
     })
     it('should keep other properties of parent', function () {
       scope.push({obj: {foo: 'FOO'}})
@@ -151,7 +156,8 @@ describe('scope', function () {
       }
       expect(fn).to.throw(/undefined variable: notdefined/)
     })
-    it('should throw when parent not defined', function () {
+    it('should throw when deep variable not exist', function () {
+      scope.set('foo', 'FOO')
       function fn () {
         scope.get('foo.bar.not.defined')
       }
@@ -209,31 +215,25 @@ describe('scope', function () {
       expect(scope.get('foo')).to.equal('zoo')
     })
   })
-
-  describe('.unshift()', function () {
-    it('should throw when trying to unshift non-object', function () {
-      expect(function () {
-        scope.unshift(false)
-      }).to.throw()
-    })
-    it('should unshift scope', function () {
-      scope.unshift({
-        foo: 'blue',
-        foo1: 'foo1'
-      })
-      expect(scope.get('foo')).to.equal('zoo')
-      expect(scope.get('foo1')).to.equal('foo1')
-    })
+  it('should pop specified scope', function () {
+    let scope1 = {
+      foo: 'foo'
+    }
+    let scope2 = {
+      bar: 'bar'
+    }
+    scope.push(scope1)
+    scope.push(scope2)
+    expect(scope.get('foo')).to.equal('foo')
+    expect(scope.get('bar')).to.equal('bar')
+    scope.pop(scope1)
+    expect(scope.get('foo')).to.equal('zoo')
+    expect(scope.get('bar')).to.equal('bar')
   })
-  describe('.shift()', function () {
-    it('should shift scope', function () {
-      scope.unshift({
-        foo: 'blue',
-        foo1: 'foo1'
-      })
-      scope.shift()
-      expect(scope.get('foo')).to.equal('zoo')
-      expect(scope.get('foo1')).to.equal(undefined)
-    })
+  it('should throw when specified scope not found', function () {
+    let scope1 = {
+      foo: 'foo'
+    }
+    expect(() => scope.pop(scope1)).to.throw('scope not found, cannot pop')
   })
 })

test/xhr.js

@@ -2,6 +2,7 @@ const Liquid = require('..')
 const sinon = require('sinon')
 const chai = require('chai')
 const expect = chai.expect
+chai.use(require('chai-as-promised'))
 
 describe('xhr', () => {
   if (process.version.match(/^v(\d+)/)[1] < 8) {