From 13b834dfb73abd8eab306c37db43978216b26444 Mon Sep 17 00:00:00 2001 From: Tim van der Lippe Date: Tue, 1 Aug 2017 21:07:19 -0700 Subject: [PATCH 01/20] Rewrite expression parser to state machine --- lib/mixins/property-effects.html | 65 +---- lib/utils/binding-parser.html | 397 +++++++++++++++++++++++++++++++ test/unit/property-effects.html | 2 +- 3 files changed, 402 insertions(+), 62 deletions(-) create mode 100644 lib/utils/binding-parser.html diff --git a/lib/mixins/property-effects.html b/lib/mixins/property-effects.html index 827c6de1a9..5d78c7cda8 100644 --- a/lib/mixins/property-effects.html +++ b/lib/mixins/property-effects.html @@ -13,6 +13,7 @@ + @@ -2627,67 +2628,9 @@ * @protected */ static _parseBindings(text, templateInfo) { - let parts = []; - let lastIndex = 0; - let m; - // Example: "literal1{{prop}}literal2[[!compute(foo,bar)]]final" - // Regex matches: - // Iteration 1: Iteration 2: - // m[1]: '{{' '[[' - // m[2]: '' '!' - // m[3]: 'prop' 'compute(foo,bar)' - while ((m = bindingRegex.exec(text)) !== null) { - // Add literal part - if (m.index > lastIndex) { - parts.push({literal: text.slice(lastIndex, m.index)}); - } - // Add binding part - let mode = m[1][0]; - let negate = Boolean(m[2]); - let source = m[3].trim(); - let customEvent = false, notifyEvent = '', colon = -1; - if (mode == '{' && (colon = source.indexOf('::')) > 0) { - notifyEvent = source.substring(colon + 2); - source = source.substring(0, colon); - customEvent = true; - } - let signature = parseMethod(source); - let dependencies = []; - if (signature) { - // Inline computed function - let {args, methodName} = signature; - for (let i=0; i + + diff --git a/test/unit/property-effects.html b/test/unit/property-effects.html index 2a7581fbb5..c272c04127 100644 --- a/test/unit/property-effects.html +++ b/test/unit/property-effects.html @@ -1252,7 +1252,7 @@ test('malformed bindings ignored', function() { el.bool = true; - assert.isTrue(el.$.boundChild.textContent.indexOf('really.long.identifier.in.malformed.binding.should.be.ignored') >= 0, true); + assert.isFalse(el.$.boundChild.textContent.indexOf('really.long.identifier.in.malformed.binding.should.be.ignored') >= 0, true); assert.isTrue(el.$.boundChild.textContent.indexOf('really.long.literal.in.malformed.binding.should.be.ignored') >= 0, true); assert.isTrue(el.$.boundChild.textContent.indexOf('3foo') >= 0, true); }); From 79d05b8a22d69d403d30a085a19705436d71ca71 Mon Sep 17 00:00:00 2001 From: Tim van der Lippe Date: Tue, 1 Aug 2017 21:11:42 -0700 Subject: [PATCH 02/20] Fix linter warnings --- lib/mixins/property-effects.html | 18 ------- lib/utils/binding-parser.html | 85 ++++++++++++++++---------------- 2 files changed, 42 insertions(+), 61 deletions(-) diff --git a/lib/mixins/property-effects.html b/lib/mixins/property-effects.html index 5d78c7cda8..032bc7797a 100644 --- a/lib/mixins/property-effects.html +++ b/lib/mixins/property-effects.html @@ -821,24 +821,6 @@ const emptyArray = []; - // Regular expressions used for binding - const IDENT = '(?:' + '[a-zA-Z_$][\\w.:$\\-*]*' + ')'; - const NUMBER = '(?:' + '[-+]?[0-9]*\\.?[0-9]+(?:[eE][-+]?[0-9]+)?' + ')'; - const SQUOTE_STRING = '(?:' + '\'(?:[^\'\\\\]|\\\\.)*\'' + ')'; - const DQUOTE_STRING = '(?:' + '"(?:[^"\\\\]|\\\\.)*"' + ')'; - const STRING = '(?:' + SQUOTE_STRING + '|' + DQUOTE_STRING + ')'; - const ARGUMENT = '(?:(' + IDENT + '|' + NUMBER + '|' + STRING + ')\\s*' + ')'; - const ARGUMENTS = '(?:' + ARGUMENT + '(?:,\\s*' + ARGUMENT + ')*' + ')'; - const ARGUMENT_LIST = '(?:' + '\\(\\s*' + - '(?:' + ARGUMENTS + '?' + ')' + - '\\)\\s*' + ')'; - const BINDING = '(' + IDENT + '\\s*' + ARGUMENT_LIST + '?' + ')'; // Group 3 - const OPEN_BRACKET = '(\\[\\[|{{)' + '\\s*'; - const CLOSE_BRACKET = '(?:]]|}})'; - const NEGATE = '(?:(!)\\s*)?'; // Group 2 - const EXPRESSION = OPEN_BRACKET + NEGATE + BINDING + CLOSE_BRACKET; - const bindingRegex = new RegExp(EXPRESSION, "g"); - /** * Create a string from binding parts of all the literal parts * diff --git a/lib/utils/binding-parser.html b/lib/utils/binding-parser.html index 43cfbdda61..06ea6b86c3 100644 --- a/lib/utils/binding-parser.html +++ b/lib/utils/binding-parser.html @@ -37,6 +37,7 @@ this.bindingData = { dependencies: [] }; + /* eslint-disable no-fallthrough */ const STATE = { INITIAL: (char, i) => { @@ -172,52 +173,49 @@ } } }, - METHOD: (binding) => { - let escaped = false; - return (char, i) => { - //console.log('METHOD') - switch (char) { - case ')': { - const methodName = this.bindingData.signature.methodName; - const dynamicFns = templateInfo.dynamicFns; - if (dynamicFns && dynamicFns[methodName] || this.bindingData.signature.static) { - this.bindingData.dependencies.push(methodName); - this.bindingData.signature.dynamicFn = true; - } - const name = text.substring(this._startChar, i).trim(); - this.bindingData.mode = binding; - if (name) { - this.bindingData.signature.args.push({ - name - }); - this.bindingData.dependencies.push(name) - } - this._storeBindingData(); - return STATE.METHODCLOSED(binding) + METHOD: (binding) => (char, i) => { + //console.log('METHOD') + switch (char) { + case ')': { + const methodName = this.bindingData.signature.methodName; + const dynamicFns = templateInfo.dynamicFns; + if (dynamicFns && dynamicFns[methodName] || this.bindingData.signature.static) { + this.bindingData.dependencies.push(methodName); + this.bindingData.signature.dynamicFn = true; } - case ',': { - const name = text.substring(this._startChar, i).trim(); - if (name) { - this.bindingData.signature.args.push({ - name - }); - this.bindingData.dependencies.push(name) - } - this._startChar = i + 1; - break; + const name = text.substring(this._startChar, i).trim(); + this.bindingData.mode = binding; + if (name) { + this.bindingData.signature.args.push({ + name + }); + this.bindingData.dependencies.push(name) } - case '\'': - case '"': { - return STATE.STRINGARG(binding, char); + this._storeBindingData(); + return STATE.METHODCLOSED(binding) + } + case ',': { + const name = text.substring(this._startChar, i).trim(); + if (name) { + this.bindingData.signature.args.push({ + name + }); + this.bindingData.dependencies.push(name) + } + this._startChar = i + 1; + break; + } + case '\'': + case '"': { + return STATE.STRINGARG(binding, char); + } + default: { + if (char >= '0' && char <= '9' || char === '-') { + return STATE.NUMBERARG(binding) } - default: { - if (char >= '0' && char <= '9' || char === '-') { - return STATE.NUMBERARG(binding) - } - if (char != ' ' && char != '\n') { - return STATE.VARIABLEARG(binding) - } + if (char != ' ' && char != '\n') { + return STATE.VARIABLEARG(binding) } } } @@ -358,6 +356,7 @@ } } } + /* eslint-enable no-fallthrough */ let state = STATE.INITIAL; let i,l; @@ -390,7 +389,7 @@ }; } - }; + } Polymer.BindingParser = BindingParser; })(); From 7eb1a627f9119fbbaade8f316db591b9dd84c457 Mon Sep 17 00:00:00 2001 From: Tim van der Lippe Date: Wed, 2 Aug 2017 14:07:15 -0700 Subject: [PATCH 03/20] Refactor to be functional and add more tests --- lib/mixins/property-effects.html | 2 +- lib/utils/binding-parser.html | 476 ++++++++++------------- test/unit/property-effects-elements.html | 10 + test/unit/property-effects.html | 30 +- 4 files changed, 240 insertions(+), 278 deletions(-) diff --git a/lib/mixins/property-effects.html b/lib/mixins/property-effects.html index 032bc7797a..15f9e5cdc7 100644 --- a/lib/mixins/property-effects.html +++ b/lib/mixins/property-effects.html @@ -2610,7 +2610,7 @@ * @protected */ static _parseBindings(text, templateInfo) { - const parserParts = new Polymer.BindingParser().parse(text, templateInfo); + const parserParts = Polymer.BindingParser.parse(text, templateInfo); if (parserParts.length) { return parserParts; } else { diff --git a/lib/utils/binding-parser.html b/lib/utils/binding-parser.html index 06ea6b86c3..fdb17fe98c 100644 --- a/lib/utils/binding-parser.html +++ b/lib/utils/binding-parser.html @@ -8,6 +8,7 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATENTS.txt --> + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/test/perf/perf-tests.html b/test/perf/perf-tests.html new file mode 100644 index 0000000000..f8da4dbe1a --- /dev/null +++ b/test/perf/perf-tests.html @@ -0,0 +1,27 @@ + + + + + + + + + + + + + Perf is go. + + + + + \ No newline at end of file From df0ee35460bbc547f21a7c543e956b7f8a0e707f Mon Sep 17 00:00:00 2001 From: Tim van der Lippe Date: Tue, 8 Aug 2017 15:46:49 -0700 Subject: [PATCH 08/20] Add documentation --- lib/utils/binding-parser.html | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/lib/utils/binding-parser.html b/lib/utils/binding-parser.html index 57babfe7b5..66545003d3 100644 --- a/lib/utils/binding-parser.html +++ b/lib/utils/binding-parser.html @@ -13,11 +13,19 @@ (function() { 'use strict'; + /** + * The open and corresponding closing brackets for surrounding bindings. + * @enum {string} + */ const BINDINGS = { '{': '}', '[': ']' }; + /** + * All states that the parser can be in. The states represent the state-machine as a whole. + * @enum {number} + */ const STATE = { INITIAL: 1, FIRSTOPENINGBINDING: 2, @@ -98,26 +106,30 @@ } /** - * Module with utilities for converting between "dash-case" and "camelCase" - * identifiers. + * Module that parses binding expressions and generates corresponding metadata. * * @namespace * @memberof Polymer - * @summary Module that provides utilities for converting between "dash-case" - * and "camelCase". + * @summary Module that parses binding expressions and generates corresponding metadata. */ const BindingParser = { + /** + * @param {string} text Text to parse from attribute or textContent + * @param {Object} templateInfo Current template metadata + * @return {Array} Array of binding part metadata + */ parse(text, templateInfo) { const parts = []; let bindingData = {}; let escaped = false; + /** @type {string} */ let quote; + /** @type {number} */ let state = STATE.INITIAL; - let i,l; - for (i=0,l=text.length; i Date: Tue, 8 Aug 2017 15:59:04 -0700 Subject: [PATCH 09/20] Fix linter warning --- lib/utils/binding-parser.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/utils/binding-parser.html b/lib/utils/binding-parser.html index 66545003d3..357d17a357 100644 --- a/lib/utils/binding-parser.html +++ b/lib/utils/binding-parser.html @@ -125,11 +125,11 @@ let escaped = false; /** @type {string} */ let quote; - /** @type {number} */ let state = STATE.INITIAL; + let i,l; - for (let i=0,l=text.length; i Date: Wed, 16 Aug 2017 19:02:55 -0700 Subject: [PATCH 10/20] Add missing dependency to bower.json --- bower.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bower.json b/bower.json index b2f988911a..b8d6992707 100644 --- a/bower.json +++ b/bower.json @@ -25,7 +25,8 @@ }, "devDependencies": { "web-component-tester": "^6.0.0", - "test-fixture": "PolymerElements/test-fixture#3.0.0-rc.1" + "test-fixture": "PolymerElements/test-fixture#3.0.0-rc.1", + "perf-tester": "polymerlabs/perf-tester" }, "private": true, "resolutions": { From 57a142367be91173c4e77096133f5632f1ffeca8 Mon Sep 17 00:00:00 2001 From: Tim van der Lippe Date: Mon, 18 Sep 2017 19:41:15 -0700 Subject: [PATCH 11/20] Extract to a mixin --- lib/mixins/property-effects.html | 85 ++- lib/utils/binding-parser.html | 640 ++++++++++++----------- test/unit/property-effects-elements.html | 8 + test/unit/property-effects.html | 66 ++- 4 files changed, 468 insertions(+), 331 deletions(-) diff --git a/lib/mixins/property-effects.html b/lib/mixins/property-effects.html index 15f9e5cdc7..b33b386b0b 100644 --- a/lib/mixins/property-effects.html +++ b/lib/mixins/property-effects.html @@ -13,7 +13,6 @@ - @@ -821,6 +820,24 @@ const emptyArray = []; + // Regular expressions used for binding + const IDENT = '(?:' + '[a-zA-Z_$][\\w.:$\\-*]*' + ')'; + const NUMBER = '(?:' + '[-+]?[0-9]*\\.?[0-9]+(?:[eE][-+]?[0-9]+)?' + ')'; + const SQUOTE_STRING = '(?:' + '\'(?:[^\'\\\\]|\\\\.)*\'' + ')'; + const DQUOTE_STRING = '(?:' + '"(?:[^"\\\\]|\\\\.)*"' + ')'; + const STRING = '(?:' + SQUOTE_STRING + '|' + DQUOTE_STRING + ')'; + const ARGUMENT = '(?:(' + IDENT + '|' + NUMBER + '|' + STRING + ')\\s*' + ')'; + const ARGUMENTS = '(?:' + ARGUMENT + '(?:,\\s*' + ARGUMENT + ')*' + ')'; + const ARGUMENT_LIST = '(?:' + '\\(\\s*' + + '(?:' + ARGUMENTS + '?' + ')' + + '\\)\\s*' + ')'; + const BINDING = '(' + IDENT + '\\s*' + ARGUMENT_LIST + '?' + ')'; // Group 3 + const OPEN_BRACKET = '(\\[\\[|{{)' + '\\s*'; + const CLOSE_BRACKET = '(?:]]|}})'; + const NEGATE = '(?:(!)\\s*)?'; // Group 2 + const EXPRESSION = OPEN_BRACKET + NEGATE + BINDING + CLOSE_BRACKET; + const bindingRegex = new RegExp(EXPRESSION, "g"); + /** * Create a string from binding parts of all the literal parts * @@ -2610,9 +2627,67 @@ * @protected */ static _parseBindings(text, templateInfo) { - const parserParts = Polymer.BindingParser.parse(text, templateInfo); - if (parserParts.length) { - return parserParts; + let parts = []; + let lastIndex = 0; + let m; + // Example: "literal1{{prop}}literal2[[!compute(foo,bar)]]final" + // Regex matches: + // Iteration 1: Iteration 2: + // m[1]: '{{' '[[' + // m[2]: '' '!' + // m[3]: 'prop' 'compute(foo,bar)' + while ((m = bindingRegex.exec(text)) !== null) { + // Add literal part + if (m.index > lastIndex) { + parts.push({literal: text.slice(lastIndex, m.index)}); + } + // Add binding part + let mode = m[1][0]; + let negate = Boolean(m[2]); + let source = m[3].trim(); + let customEvent = false, notifyEvent = '', colon = -1; + if (mode == '{' && (colon = source.indexOf('::')) > 0) { + notifyEvent = source.substring(colon + 2); + source = source.substring(0, colon); + customEvent = true; + } + let signature = parseMethod(source); + let dependencies = []; + if (signature) { + // Inline computed function + let {args, methodName} = signature; + for (let i=0; i + \ No newline at end of file diff --git a/lib/utils/binding-parser.html b/lib/utils/binding-parser.html index 357d17a357..0e614d6a2b 100644 --- a/lib/utils/binding-parser.html +++ b/lib/utils/binding-parser.html @@ -9,117 +9,151 @@ --> + + diff --git a/test/unit/property-effects.html b/test/unit/property-effects.html index bef3730a68..40f93aa5c0 100644 --- a/test/unit/property-effects.html +++ b/test/unit/property-effects.html @@ -358,30 +358,6 @@ assert.equal(el.$.boundWithDash.textContent, 'yes'); }); - test('binding with slash', function() { - el.objectWithSlash = { - 'binding/with/slash': 'yes' - }; - assert.equal(el.$.boundWithSlash.textContent, 'yes'); - }); - - test('json should not be a binding', function() { - assert.equal(el.$.jsonContent.textContent, '[["Jan", 31],["Feb", 28],["Mar", 31]]'); - }); - - test('binding with non-English unicode', function() { - el.objectWithNonEnglishUnicode = { - 商品名: 'yes' - }; - assert.equal(el.$.nonEnglishUnicode.textContent, 'yes'); - }); - - test('binding with booleans', function() { - el.otherValue = 10; - assert.equal(el.$.booleanTrue.textContent, 'foo(field, true): 10'); - assert.equal(el.$.booleanFalse.textContent, 'foo(field, false): 20'); - }); - test('class attribute without template scope not erased', function() { var el = document.querySelector('.class1'); assert.notEqual(el, null, 'class without template scope is undefined'); @@ -443,6 +419,48 @@ assert.equal(el.__observerCalled, 1); }); }); + + suite('can work with binding parser', function() { + setup(function() { + document.body.removeChild(el); + el = document.createElement('x-basic-binding-parser'); + document.body.appendChild(el); + }); + + test('binding with slash', function() { + el.objectWithSlash = { + 'binding/with/slash': 'yes' + }; + assert.equal(el.$.boundWithSlash.textContent, 'yes'); + }); + + test('json should not be a binding', function() { + assert.equal(el.$.jsonContent.textContent, '[["Jan", 31],["Feb", 28],["Mar", 31]]'); + }); + + test('binding with non-English unicode', function() { + el.objectWithNonEnglishUnicode = { + 商品名: 'yes' + }; + assert.equal(el.$.nonEnglishUnicode.textContent, 'yes'); + }); + + test('binding with booleans', function() { + el.otherValue = 10; + assert.equal(el.$.booleanTrue.textContent, 'foo(field, true): 10'); + assert.equal(el.$.booleanFalse.textContent, 'foo(field, false): 20'); + }); + + suite('equivalent behavior as regex', function() { + // Loop over the suite "single-element binding effects" (parent of the parent of this suite) + // And make sure that the tests there also pass on the binding-parser + // + // t.title is the name of the test and t.fn contains the test body + this.parent.parent.tests.forEach(t => { + test(t.title, t.fn); + });; + }); + }); }); suite('computed bindings with dynamic functions', function() { From d8cf449ee52d11fcb04d970611a42c52c20715dc Mon Sep 17 00:00:00 2001 From: Tim van der Lippe Date: Mon, 18 Sep 2017 19:49:29 -0700 Subject: [PATCH 12/20] Fix linter errors --- lib/utils/binding-parser.html | 10 +++++----- test/unit/property-effects.html | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/utils/binding-parser.html b/lib/utils/binding-parser.html index 0e614d6a2b..6bff5f35dd 100644 --- a/lib/utils/binding-parser.html +++ b/lib/utils/binding-parser.html @@ -44,7 +44,7 @@ VARIABLEARG: 13, METHODCLOSED: 14, METHODCLOSEDBINDING: 15 - } + }; function pushLiteral(text, i, parts, startChar) { const literal = text.substring(startChar || 0, i); @@ -83,7 +83,7 @@ } else { const arg = { name - } + }; arg.structured = Polymer.Path.isPath(name); if (arg.structured) { arg.wildcard = (name.slice(-2) == '.*'); @@ -92,7 +92,7 @@ } } bindingData.signature.args.push(arg); - bindingData.dependencies.push(name) + bindingData.dependencies.push(name); bindingData.signature.static = false; } } @@ -284,7 +284,7 @@ break; } case ',': { - storeMethodVariable(bindingData, text, i) + storeMethodVariable(bindingData, text, i); bindingData.startChar = i + 1; break; } @@ -396,7 +396,7 @@ return null; } - } + }; }); Polymer.BindingParser = BindingParser; diff --git a/test/unit/property-effects.html b/test/unit/property-effects.html index 40f93aa5c0..2a567e335c 100644 --- a/test/unit/property-effects.html +++ b/test/unit/property-effects.html @@ -458,7 +458,7 @@ // t.title is the name of the test and t.fn contains the test body this.parent.parent.tests.forEach(t => { test(t.title, t.fn); - });; + }); }); }); }); From 19d4b8cb9c169567c9b67c05ddc4bf3918aa5e3c Mon Sep 17 00:00:00 2001 From: Tim van der Lippe Date: Fri, 22 Sep 2017 16:03:41 -0700 Subject: [PATCH 13/20] Rename to StrictBindingParser --- .../{binding-parser.html => strict-binding-parser.html} | 7 ++++--- test/unit/property-effects-elements.html | 8 ++++---- test/unit/property-effects.html | 4 ++-- 3 files changed, 10 insertions(+), 9 deletions(-) rename lib/utils/{binding-parser.html => strict-binding-parser.html} (98%) diff --git a/lib/utils/binding-parser.html b/lib/utils/strict-binding-parser.html similarity index 98% rename from lib/utils/binding-parser.html rename to lib/utils/strict-binding-parser.html index 6bff5f35dd..5b94efe5b9 100644 --- a/lib/utils/binding-parser.html +++ b/lib/utils/strict-binding-parser.html @@ -110,13 +110,14 @@ /** * Mixin that parses binding expressions and generates corresponding metadata. * The implementation is different than in `property-effects`, as it uses a - * state machine instead of a regex. + * state machine instead of a regex. As such, this implementation is able to + * handle more cases, with the potential performance hit. * * @namespace * @memberof Polymer * @summary Mixin that parses binding expressions and generates corresponding metadata. */ - const BindingParser = Polymer.dedupingMixin((base) => { + const StrictBindingParser = Polymer.dedupingMixin((base) => { return class extends base { @@ -399,6 +400,6 @@ }; }); - Polymer.BindingParser = BindingParser; + Polymer.StrictBindingParser = StrictBindingParser; })(); diff --git a/test/unit/property-effects-elements.html b/test/unit/property-effects-elements.html index 8defcc9519..622d128f55 100644 --- a/test/unit/property-effects-elements.html +++ b/test/unit/property-effects-elements.html @@ -7,7 +7,7 @@ Code distributed by Google as part of the polymer project is also subject to an additional IP rights grant found at http://polymer.github.io/PATENTS.txt --> - +