fix expression parser setter

benchmarks/bench.js

@@ -1,3 +1,5 @@
 require('./observer').run(function () {
-  require('./instantiation').run()
+  require('./instantiation').run(function () {
+    require('./expression')
+  })
 })

benchmarks/expression.js

@@ -0,0 +1,35 @@
+console.log('\Expression Parser\n')
+
+var Cache = require('../src/cache')
+var parse = require('../src/parse/expression').parse
+
+Cache.prototype.get = Cache.prototype.put = function () {}
+
+function bench (id, fn) {
+  var s = Date.now()
+  var max = i = 10000
+  while (i--) {
+    fn()
+  }
+  var used = Date.now() - s
+  var ops = Math.round(16 / (used / max))
+  console.log(id + ': ' + ops + ' ops/frame')
+}
+
+var side
+
+bench('simple path', function () {
+  side = parse('a.b.c')
+})
+
+bench('complex path', function () {
+  side = parse('a["b"].c')
+})
+
+bench('simple exp', function () {
+  side = parse('a.b + c')
+})
+
+bench('complex exp', function () {
+  side = parse('a.b + c')
+})

src/parse/expression.js

@@ -89,10 +89,11 @@ function restore (str, i) {
  * and generate getter/setter functions.
  *
  * @param {String} exp
+ * @param {Boolean} needSet
  * @return {Function}
  */
 
-function compileExpFns (exp) {
+function compileExpFns (exp, needSet) {
   // reset state
   saved.length = 0
   paths = []
@@ -106,10 +107,13 @@ function compileExpFns (exp) {
   body = (' ' + body)
     .replace(pathReplaceRE, rewrite)
     .replace(restoreRE, restore)
-  var getter = makeGetter(exp, body)
+  var getter = makeGetter(body)
   if (getter) {
+    getter.body = body
     getter.paths = paths
-    getter.setter = makeSetter(body)
+    if (needSet) {
+      getter.setter = makeSetter(body)
+    }
   }
   return getter
 }
@@ -134,6 +138,7 @@ function compilePathFns (exp) {
   }
   // save root path segment
   getter.paths = [exp.match(rootPathRE)[0]]
+  // always generate setter for simple paths
   getter.setter = function (obj, val) {
     Path.set(obj, path, val)
   }
@@ -150,11 +155,11 @@ function compilePathFns (exp) {
  * @return {Function|undefined}
  */
 
-function makeGetter (exp, body) {
+function makeGetter (body) {
   try {
     return new Function('scope', 'return ' + body + ';')
   } catch (e) {
-    _.warn('Invalid expression: "' + exp + '\nGenerated function body: ' + body)
+    _.warn('Invalid expression. Generated function body: ' + body)
   }
 }
 
@@ -164,9 +169,6 @@ function makeGetter (exp, body) {
  * This is only needed in rare situations like "a[b]" where
  * a settable path requires dynamic evaluation.
  *
- * Not doing try-catch here because this only gets called
- * if makeGetter() worked.
- *
  * This setter function may throw error when called if the
  * expression body is not a valid left-hand expression in
  * assignment.
@@ -176,20 +178,40 @@ function makeGetter (exp, body) {
  */
 
 function makeSetter (body) {
-  return new Function('scope', 'value', body + ' = value;')
+  try {
+    return new Function('scope', 'value', body + ' = value;')
+  } catch (e) {
+    _.warn('Invalid setter function body: ' + body)
+  }
+}
+
+/**
+ * Check for setter existence on a cache hit.
+ *
+ * @param {Function} fn
+ */
+
+function checkSetter (fn) {
+  if (!fn.setter) {
+    fn.setter = makeSetter(fn.body)
+  }
 }
 
 /**
  * Parse an expression and rewrite into a getter/setter functions
  *
  * @param {String} exp
+ * @param {Boolean} needSet
  * @return {Function}
  */
 
-exports.parse = function (exp) {
+exports.parse = function (exp, needSet) {
   // try cache
   var hit = expressionCache.get(exp)
   if (hit) {
+    if (needSet) {
+      checkSetter(hit)
+    }
     return hit
   }
   exp = exp.trim()
@@ -198,7 +220,7 @@ exports.parse = function (exp) {
   // that's too rare and we don't care.
   var getter = pathTestRE.test(exp)
     ? compilePathFns(exp)
-    : compileExpFns(exp)
+    : compileExpFns(exp, needSet)
   expressionCache.put(exp, getter)
   return getter
 }

test/unit/specs/expression_parser_spec.js

@@ -172,7 +172,7 @@ describe('Expression Parser', function () {
   })
 
   it('dynamic setter', function () {
-    var setter = expParser.parse('a[b]').setter
+    var setter = expParser.parse('a[b]', true).setter
     var scope = {
       a: { c: 1 },
       b: 'c'
@@ -182,7 +182,7 @@ describe('Expression Parser', function () {
   })
 
   it('simple path setter', function () {
-    var setter = expParser.parse('a.b.c').setter
+    var setter = expParser.parse('a.b.c', true).setter
     var scope = {}
     expect(function () {
       setter(scope, 123)