2.0 Improvements to binding API #4510

kevinpschaaf · 2017-04-06T22:27:43Z

Adds override points for _parseBindings and _evaluateBinding
Adds support for runtime template binding
Moves ready(), _hasAccessor tracking, and instance property swizzle at ready time to PropertyAccessors
Other refactoring and cleanup

arthurevans · 2017-04-06T23:21:17Z

I am so excite! And also confuse!

How can people do runtime template binding? By that do you mean identifying an instance-specific template and then calling _bindTemplate on it? (I note there's an addBinding method, but AFAICT that is private and not exposed on PropertyEffects.)

Can you bind a template more than once? (i.e., could you mutate and re-bind the template) Can you bind multiple templates?

Inquiring minds want to know...

Thanks,
Arthur

sorvell · 2017-04-06T23:45:53Z

lib/mixins/property-accessors.html

-        this.__dataCounter++;
-        this._propertiesChanged(this.__data, changedProps, oldProps);
-        this.__dataCounter--;
+        if (!this.__dataInitialized) {


Now that we have __dataInitialized it probably makes sense to prevent _invalidateProperties from scheduling a flush if this flag is not true.

sorvell · 2017-04-07T01:02:35Z

lib/mixins/property-effects.html

+          part.compoundIndex = i;
+          for (let j=0; j<dependencies.length; j++) {
+            let trigger = dependencies[j];
+            if (typeof trigger == 'string') {


Bookmark to consider if we want to assume we should parseArg the trigger if it's a string.

sorvell · 2017-04-07T01:05:51Z

lib/mixins/property-effects.html

-    if (hasPaths && (path.length > info.value.length) &&
-        (info.kind == 'property') && !info.isCompound &&
-        node.__propertyEffects && node.__propertyEffects[info.name]) {
+    if (hasPaths && part.source && (path.length > part.source.length) &&


Add a comment explaining why path processing is not included with _evaluateBinding.

sorvell · 2017-04-07T01:43:32Z

lib/mixins/property-effects.html

+       * @protected
+       */
+      _stampBoundTemplate(template) {
+        let dom = super._stampTemplate(template);


super => this

sorvell · 2017-04-07T01:52:38Z

lib/mixins/property-effects.html

+            }
+          } else {
+            // Property or path
+            dependencies.push(source);


Should parseArg(source) so it doesn't auto-wildcard.

kevinpschaaf · 2017-04-07T19:07:14Z

@arthurevans You can call _stampBoundTemplate(template) on a template (with bindings, events, etc.) of your choosing as many times as you like, and the doc frag returned will have elements that are bound to the element. _bindTemplate is only for prototypical binding, not runtime binding (_stampBoundTemplate stamps & binds) for more efficient stamping of the element's main template -- don't expect end users to use that unless they're building their own base classes since ElementMixin does this for you already.

_stampBoundTemplate doesn't support mutating the template and stamping it again... for that you'd just need to clone the template before mutating it, since template metadata from parsing it are memoized on the template. Should probably add that limitation to the docs. There's also a limitation with shadycss that I did mention in there, which is that since shadycss doesn't support runtime addition of <style>s, any runtime-stamped templates can't stamp new styles-- their styles need to be in the main (prototypical) template for the element.

Note one review comment is to change _stampBoundTemplate back to an override of _stampTemplate, so the name will likely change back.

This PR (and the previously merged #4432) wasn't outright intending to add new features in the RC; the main goal here was to refactor the existing code to make the whole binding system more extensible,however the ability to stamp multiple templates sort of fell out pretty easily, so we'll probably go ahead and add this feature before the final release.

Still need to add tests and address review feedback... If anything starts looking too risky we may trim this back. Will keep you in the loop.

- Adds override points for _parseBindings and _evaluateBinding - Adds support for runtime template binding - Moves ready(), _hasAccessor tracking, and instance property swizzle at ready time to PropertyAccessors

…o reference.

* PropertyAccessors must call `_flushProperties` to enable * Avoid tearing off oldProps (unnecessary) * Add `addBinding` docs * Merge notifyListeners into `setupBindings` * Add comment re: path-bindings not being overridable * Remove `dom` argument from `_bindTemplate` * Rename `_stampBoundTemplate` back to `_stampTemplate`

* Refactor `_bindTemplate` to remove problematic `hasCreatedAccessors` * Remove vestigial `dom` from `_bindTemplate` call * Rename `_unstampTemplate` to `_removeBoundDom` * Add `infoIndex` to `nodeInfo` (and renamed parent & index) * Add test to ensure runtime accessors created for new props in runtime stamped template * Changed custom binding test to use different prop names * Added test for #first count after removing bound dom

googlebot added the cla: yes label Apr 6, 2017

kevinpschaaf requested a review from sorvell April 6, 2017 22:27

kevinpschaaf assigned kevinpschaaf and sorvell and unassigned sorvell Apr 6, 2017

sorvell reviewed Apr 6, 2017

View reviewed changes

sorvell reviewed Apr 7, 2017

View reviewed changes

kevinpschaaf added 3 commits April 7, 2017 16:12

Improvements to binding API:

ea4e7d9

- Adds override points for _parseBindings and _evaluateBinding - Adds support for runtime template binding - Moves ready(), _hasAccessor tracking, and instance property swizzle at ready time to PropertyAccessors

Fix _hasAccessor for readOnly. Collapse addBinding & addBindingEffects

396c102

Put $ on dom, and assign to element as needed. Eliminate _templateInf…

03bed19

…o reference.

kevinpschaaf force-pushed the 2.0-binding-api-refactor branch from ed30b21 to 03bed19 Compare April 7, 2017 23:13

kevinpschaaf and others added 13 commits April 7, 2017 19:14

Add initial runtime stamping tests.

eb6ab63

Added tests for custom parsing, effects, and binding.

1cf955b

Add tests for adding/removing runtime property effects.

1471106

Ensure prototype wasn't affected by runtime effects.

bf2dbe0

Fix lint error.

dff5f2b

Merge branch 'master' into 2.0-binding-api-refactor

cd321a3

Add more HTMLImports.whenReady

bc71318

use chrome beta

0724f18

alias another way

d297047

[ci skip] Fix comment.

becd1d3

Add more comments

d0bd96d

kevinpschaaf force-pushed the 2.0-binding-api-refactor branch from 39a455e to d0bd96d Compare April 13, 2017 20:59

kevinpschaaf added 2 commits April 13, 2017 15:56

Fix perf regressions.

86e35e3

Eliminate rest args for better perf on stable chrome.

fa67457

sorvell approved these changes Apr 13, 2017

View reviewed changes

sorvell merged commit 9997a04 into master Apr 13, 2017

sorvell deleted the 2.0-binding-api-refactor branch April 13, 2017 23:41

kevinpschaaf mentioned this pull request Apr 25, 2017

Provide hooks to process custom template annotations #3369

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

2.0 Improvements to binding API #4510

2.0 Improvements to binding API #4510

kevinpschaaf commented Apr 6, 2017

arthurevans commented Apr 6, 2017

sorvell Apr 6, 2017

sorvell Apr 7, 2017

sorvell Apr 7, 2017

sorvell Apr 7, 2017

sorvell Apr 7, 2017

kevinpschaaf commented Apr 7, 2017

2.0 Improvements to binding API #4510

2.0 Improvements to binding API #4510

Conversation

kevinpschaaf commented Apr 6, 2017

arthurevans commented Apr 6, 2017

sorvell Apr 6, 2017

Choose a reason for hiding this comment

sorvell Apr 7, 2017

Choose a reason for hiding this comment

sorvell Apr 7, 2017

Choose a reason for hiding this comment

sorvell Apr 7, 2017

Choose a reason for hiding this comment

sorvell Apr 7, 2017

Choose a reason for hiding this comment

kevinpschaaf commented Apr 7, 2017