improve param attributes

yyx990803 · yyx990803 · commit 6d0697b2e409 · 2015-05-11T11:16:20.000-04:00
- support multiple mustache tags
- support filters
- support arbitrary expression
- support explicit one-way binding syntax {{*parentKey}}
- non-settable expressions are automatically one-way
diff --git a/component.json b/component.json
@@ -51,6 +51,7 @@
     "src/instance/compile.js",
     "src/instance/events.js",
     "src/instance/init.js",
+    "src/instance/misc.js",
     "src/instance/scope.js",
     "src/observer/array.js",
     "src/observer/dep.js",
diff --git a/src/compiler/compile.js b/src/compiler/compile.js
@@ -418,18 +418,9 @@ function compileParamAttributes (el, attrs, paramNames, options) {
           el.removeAttribute(name)
         }
         attrs[name] = null
-        if (tokens.length > 1) {
-          _.warn(
-            'Invalid param attribute binding: "' +
-            name + '="' + value + '"' +
-            '\nDon\'t mix binding tags with plain text ' +
-            'in param attribute bindings.'
-          )
-          continue
-        } else {
-          param.dynamic = true
-          param.value = tokens[0].value
-        }
+        param.dynamic = true
+        param.value = textParser.tokensToExp(tokens)
+        param.oneTime = tokens.length === 1 && tokens[0].oneTime
       }
       params.push(param)
     }
@@ -459,14 +450,18 @@ function makeParamsLinkFn (params, options) {
       // so we need to wrap the path here
       path = _.camelize(param.name.replace(dataAttrRE, ''))
       if (param.dynamic) {
-        // dynamic param attribtues are bound as v-with.
-        // we can directly duck the descriptor here beacuse
-        // param attributes cannot use expressions or
-        // filters.
-        vm._bindDir('with', el, {
-          arg: path,
-          expression: param.value
-        }, def)
+        if (param.oneTime) {
+          vm.$set(path, vm.$parent.$get(param.value))
+        } else {
+          // dynamic param attribtues are bound as v-with.
+          // we can directly duck the descriptor here beacuse
+          // param attributes cannot use expressions or
+          // filters.
+          vm._bindDir('with', el, {
+            arg: path,
+            expression: param.value
+          }, def)
+        }
       } else {
         // just set once
         vm.$set(path, param.value)
diff --git a/src/directive.js b/src/directive.js
@@ -141,7 +141,7 @@ p._checkStatement = function () {
   var expression = this.expression
   if (
     expression && this.acceptStatement &&
-    !expParser.pathTestRE.test(expression)
+    !expParser.isSimplePath(expression)
   ) {
     var fn = expParser.parse(expression).get
     var vm = this.vm
diff --git a/src/directives/with.js b/src/directives/with.js
@@ -63,16 +63,20 @@ module.exports = {
       // immediately.
       child.$set(childKey, this.parentWatcher.value)
 
-      this.childWatcher = new Watcher(
-        child,
-        childKey,
-        function (val) {
-          if (!locked) {
-            lock()
-            parent.$set(parentKey, val)
+      // only setup two-way binding if the parentKey is
+      // a "settable" simple path.
+      if (expParser.isSimplePath(parentKey)) {
+        this.childWatcher = new Watcher(
+          child,
+          childKey,
+          function (val) {
+            if (!locked) {
+              lock()
+              parent.$set(parentKey, val)
+            }
           }
-        }
-      )
+        )
+      }
     }
   },
 
diff --git a/src/instance/misc.js b/src/instance/misc.js
@@ -0,0 +1,18 @@
+var _ = require('../util')
+
+/**
+ * Apply a filter to a list of arguments.
+ * This is only used internally inside expressions with
+ * inlined filters.
+ *
+ * @param {String} id
+ * @param {Array} args
+ * @return {*}
+ */
+
+exports._applyFilter = function (id, args) {
+  var registry = this.$options.filters
+  var filter = registry[id]
+  _.assertAsset(filter, 'filter', id)
+  return (filter.read || filter).apply(this, args)
+}
diff --git a/src/parsers/expression.js b/src/parsers/expression.js
@@ -239,17 +239,24 @@ exports.parse = function (exp, needSet) {
   // but that's too rare and we don't care.
   // also skip boolean literals and paths that start with
   // global "Math"
-  var res =
-    pathTestRE.test(exp) &&
-    // don't treat true/false as paths
-    !booleanLiteralRE.test(exp) &&
-    // Math constants e.g. Math.PI, Math.E etc.
-    exp.slice(0, 5) !== 'Math.'
-      ? compilePathFns(exp)
-      : compileExpFns(exp, needSet)
+  var res = exports.isSimplePath(exp)
+    ? compilePathFns(exp)
+    : compileExpFns(exp, needSet)
   expressionCache.put(exp, res)
   return res
 }
 
-// Export the pathRegex for external use
-exports.pathTestRE = pathTestRE
+/**
+ * Check if an expression is a simple path.
+ *
+ * @param {String} exp
+ * @return {Boolean}
+ */
+
+exports.isSimplePath = function (exp) {
+  return pathTestRE.test(exp) &&
+    // don't treat true/false as paths
+    !booleanLiteralRE.test(exp) &&
+    // Math constants e.g. Math.PI, Math.E etc.
+    exp.slice(0, 5) !== 'Math.'
+}
diff --git a/src/parsers/text.js b/src/parsers/text.js
@@ -136,9 +136,7 @@ function formatToken (token, vm, single) {
   return token.tag
     ? vm && token.oneTime
       ? '"' + vm.$eval(token.value) + '"'
-      : single
-        ? token.value
-        : inlineFilters(token.value)
+      : inlineFilters(token.value, single)
     : '"' + token.value + '"'
 }
 
@@ -151,13 +149,16 @@ function formatToken (token, vm, single) {
  * to directive parser and watcher mechanism.
  *
  * @param {String} exp
+ * @param {Boolean} single
  * @return {String}
  */
 
 var filterRE = /[^|]\|[^|]/
-function inlineFilters (exp) {
+function inlineFilters (exp, single) {
   if (!filterRE.test(exp)) {
-    return '(' + exp + ')'
+    return single
+      ? exp
+      : '(' + exp + ')'
   } else {
     var dir = dirParser.parse(exp)[0]
     if (!dir.filters) {
@@ -169,9 +170,7 @@ function inlineFilters (exp) {
         var args = filter.args
           ? ',"' + filter.args.join('","') + '"'
           : ''
-        filter = 'this.$options.filters["' + filter.name + '"]'
-        exp = '(' + filter + '.read||' + filter + ')' +
-          '.apply(this,[' + exp + args + '])'
+        exp = 'this._applyFilter("' + filter.name + '",[' + exp + args + '])'
       }
       return exp
     }
diff --git a/src/vue.js b/src/vue.js
@@ -70,6 +70,7 @@ extend(p, require('./instance/init'))
 extend(p, require('./instance/events'))
 extend(p, require('./instance/scope'))
 extend(p, require('./instance/compile'))
+extend(p, require('./instance/misc'))
 
 /**
  * Mixin public API methods
diff --git a/test/unit/specs/compiler/compile_spec.js b/test/unit/specs/compiler/compile_spec.js
@@ -29,6 +29,12 @@ if (_.inBrowser) {
         },
         $interpolate: function (value) {
           return data[value]
+        },
+        $parent: {
+          _directives: [],
+          $get: function (v) {
+            return 'from parent: ' + v
+          }
         }
       }
       spyOn(vm, '_bindDir').and.callThrough()
@@ -151,27 +157,54 @@ if (_.inBrowser) {
     it('param attributes', function () {
       var options = merge(Vue.options, {
         _asComponent: true,
-        paramAttributes: ['a', 'data-some-attr', 'some-other-attr', 'invalid', 'camelCase']
+        paramAttributes: [
+          'a',
+          'data-some-attr',
+          'some-other-attr',
+          'multiple-attrs',
+          'onetime',
+          'with-filter',
+          'camelCase'
+        ]
       })
       var def = Vue.options.directives['with']
       el.setAttribute('a', '1')
       el.setAttribute('data-some-attr', '{{a}}')
       el.setAttribute('some-other-attr', '2')
-      el.setAttribute('invalid', 'a {{b}} c') // invalid
+      el.setAttribute('multiple-attrs', 'a {{b}} c')
+      el.setAttribute('onetime', '{{*a}}')
+      el.setAttribute('with-filter', '{{a | filter}}')
       transclude(el, options)
       var linker = compile(el, options)
       linker(vm, el)
-      // should skip literal & invliad
-      expect(vm._bindDir.calls.count()).toBe(1)
+      // should skip literals and one-time bindings
+      expect(vm._bindDir.calls.count()).toBe(3)
+      // data-some-attr
       var args = vm._bindDir.calls.argsFor(0)
       expect(args[0]).toBe('with')
       expect(args[1]).toBe(null)
       expect(args[2].arg).toBe('someAttr')
+      expect(args[2].expression).toBe('a')
+      expect(args[3]).toBe(def)
+      // multiple-attrs
+      args = vm._bindDir.calls.argsFor(1)
+      expect(args[0]).toBe('with')
+      expect(args[1]).toBe(null)
+      expect(args[2].arg).toBe('multipleAttrs')
+      expect(args[2].expression).toBe('"a "+(b)+" c"')
       expect(args[3]).toBe(def)
-      // invalid and camelCase should've warn
-      expect(_.warn.calls.count()).toBe(2)
-      // literal should've called vm.$set
+      // with-filter
+      args = vm._bindDir.calls.argsFor(2)
+      expect(args[0]).toBe('with')
+      expect(args[1]).toBe(null)
+      expect(args[2].arg).toBe('withFilter')
+      expect(args[2].expression).toBe('this._applyFilter("filter",[a])')
+      expect(args[3]).toBe(def) 
+      // camelCase should've warn
+      expect(_.warn.calls.count()).toBe(1)
+      // literal and one time should've called vm.$set
       expect(vm.$set).toHaveBeenCalledWith('a', '1')
+      expect(vm.$set).toHaveBeenCalledWith('onetime', 'from parent: a')
       expect(vm.$set).toHaveBeenCalledWith('someOtherAttr', '2')
     })
 
diff --git a/test/unit/specs/instance/misc_spec.js b/test/unit/specs/instance/misc_spec.js
@@ -0,0 +1,22 @@
+var Vue = require('../../../../src/vue')
+
+describe('misc', function () {
+
+  it('_applyFilter', function () {
+    var vm = new Vue({
+      filters: {
+        a: {
+          read: function (a, b) {
+            return a + b
+          }
+        },
+        b: function (a, b) {
+          return a - b
+        }
+      }
+    })
+    expect(vm._applyFilter('a', [1, 1])).toBe(2)
+    expect(vm._applyFilter('b', [1, 1])).toBe(0)
+  })
+
+})
diff --git a/test/unit/specs/parsers/text_spec.js b/test/unit/specs/parsers/text_spec.js
@@ -101,6 +101,14 @@ describe('Text Parser', function () {
     expect(exp).toBe('"view-"+(test + 1)+"-test-"+(ok + "|")')
   })
 
+  it('tokens to expression, single expression', function () {
+    var tokens = textParser.parse('{{test}}')
+    var exp = textParser.tokensToExp(tokens)
+    // should not have parens so it can be treated as a
+    // simple path by the expression parser
+    expect(exp).toBe('test')
+  })
+
   it('tokens to expression with oneTime tags & vm', function () {
     var vm = new Vue({
       data: { test: 'a', ok: 'b' }
@@ -110,16 +118,10 @@ describe('Text Parser', function () {
     expect(exp).toBe('"view-"+"a"+"-test-"+(ok)')
   })
 
-  it('tokens to expression with filters, single expression', function () {
-    var tokens = textParser.parse('{{test | abc}}')
-    var exp = textParser.tokensToExp(tokens)
-    expect(exp).toBe('test | abc')
-  })
-
   it('tokens to expression with filters, multiple expressions', function () {
-    var tokens = textParser.parse('a {{b | c d}} e')
+    var tokens = textParser.parse('a {{b | c d | f}} e')
     var exp = textParser.tokensToExp(tokens)
-    expect(exp).toBe('"a "+(this.$options.filters["c"].read||this.$options.filters["c"]).apply(this,[b,"d"])+" e"')
+    expect(exp).toBe('"a "+this._applyFilter("f",[this._applyFilter("c",[b,"d"])])+" e"')
   })
 
 })
