Merge #60: Fix problems with skipped _notifyPath forwarding

Starcounter · Oct 19, 2017 · a4ea7d9 · a4ea7d9
2 parents 8725dff + 41ba47c
commit a4ea7d9
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 18 deletions.
diff --git a/starcounter-include.html b/starcounter-include.html
@@ -227,13 +227,14 @@
                 }
             }
             // do not change if there is one, it's same as before, and we are not forced
+            if (!compositionString) {
+                composition = this.defaultComposition && this.defaultComposition.cloneNode(true);
+                compositionString = this.defaultComposition;
+            } else {
+                // make clone to avoid direct binding
+                composition = this.stringToDocumentFragment(compositionString);
+            }
             if (!compositionString || compositionString !== this._lastLayout || this._forceLayoutChange) {
-                if (!compositionString) {
-                    composition = this.defaultComposition && this.defaultComposition.cloneNode(true);
-                } else {
-                    // make clone to avoid direct binding
-                    composition = this.stringToDocumentFragment(compositionString);
-                }
                 this.stampComposition(composition);
                 this._forceLayoutChange = false;
                 this._lastLayout = compositionString;
@@ -246,6 +247,7 @@
          * @param  {Mixed} value New value
          */
         StarcounterIncludePrototype._notifyPath = function(path, value) {
+            var modelAlreadyLoaded = this.template.model == this.partial;
             if (path.indexOf("partial.") === 0 || path.indexOf("viewModel.") === 0) {
                 if (
                     // path === "viewModel.Html" || // covered by  viewModel._whatever_
@@ -259,11 +261,6 @@
                     (path.indexOf('.Html') === path.length - 5) && path.slice(8, -5).indexOf('.') === -1
                 ) {
                     this.partialChanged(this.partial);
-                } else if(
-                    // if viewModel|partial._whatever_, but not _ver - scope changed
-                    path.split('.').length === 2 && path.indexOf('_ver#s') === -1 && path.indexOf('_ver#c$') === -1
-                ){
-                    this.partialChanged(this.partial);
                 } else {
                     const compositionProviderPath = findCompositionProviderPath(this.partial);
                     // if (path === 'partial.' + compositionProviderPath ||
@@ -280,11 +277,15 @@
                     } else if (path === 'partial.' + compositionProviderPath + '.PartialId' || path === 'viewModel.' + compositionProviderPath + '.PartialId') {
                         const compositionProvider = findCompositionProvider(this.partial);
                         this.partialId = compositionProvider && compositionProvider.PartialId; //should always be string
-                    } else if (this.template._notifyPath) {
-                        this.template._notifyPath(
-                            path.replace("partial.", "model.")
-                                .replace("viewModel.", "model."),
-                             value);
+                    } else {
+                        this.partialChanged(this.partial);
+
+                        if (modelAlreadyLoaded && this.template._notifyPath) {
+                            this.template._notifyPath(
+                                path.replace("partial.", "model.")
+                                    .replace("viewModel.", "model."),
+                                 value);
+                        }
                     }
                 }
             }

diff --git a/test/composition/declarative-shadow-dom.html b/test/composition/declarative-shadow-dom.html
@@ -80,6 +80,17 @@ <h2>Custom Shadow DOM composition</h2>
                     .equal(document.querySelector(REFERENCE_COMPOSITION).innerHTML.trim());
             });
         });
+        describe('after partial property is replaced with itself', function () {
+            it('should keep using the existing composition', function (done) { //otherwise ViewKeeper (aka workspaces) do not keep state in Firefox and Edge
+                var old  = scInclude.shadowRoot.querySelector("h2");
+                scInclude.partial = scInclude.partial;
+                setTimeout(function() {
+                    expect(old).to.be.not.null;
+                    expect(old).to.be.equal(scInclude.shadowRoot.querySelector("h2"));
+                    done();
+                }, 500);
+            });
+        });
 
     });
     </script>

diff --git a/test/partialAttribute/nested-html/stamped_from_polymer_template.html b/test/partialAttribute/nested-html/stamped_from_polymer_template.html
@@ -92,11 +92,12 @@
                     done();
                 });
             });
-            it('should forward Polymer notification to `imported-template`', function () {
+
+            it('should NOT forward Polymer notification to `imported-template` before it was stamped', function () {
                 sinon.stub(importedTemplate, "_notifyPath");
                 var newVal = "notification works";
                 domBind.set("model.Page.scope1.doesItWork", newVal);
-                expect(importedTemplate._notifyPath).to.be.calledWith("model.scope1.doesItWork", newVal)
+                expect(importedTemplate._notifyPath).to.be.not.calledWith("model.scope1.doesItWork", newVal)
             });
 
             describe('after `dom-bind`` changed the partial property', function () {
@@ -183,6 +184,26 @@
                     });
                 });
             });
+
+            describe('after `dom-bind` re-set the sub-partial property (just one scope) to the same instance but potentially with new values', function () {
+                var scope;
+
+                beforeEach(function(done) {
+                    // IDEA: promisify juicy-html
+                    sinon.stub(importedTemplate, "_notifyPath");
+                    importedTemplate.addEventListener('stamping', function onceStamped(){
+                        importedTemplate.removeEventListener('stamping', onceStamped);
+                        scope = domBind.get('model.Page.scope1');
+                        domBind.set('model.Page.scope1', scope);
+                        done();
+                    });
+                });
+
+                it('should forward the notification to `imported-template`', function () {
+                    expect(importedTemplate._notifyPath).to.be.calledWith("model.scope1", scope)
+                });
+            });
+
             describe('after `dom-bind`` changed the sub-partial property (just one scope) to new one with thethat results with the same merged Html', function () {
                 var changedScopeWithSameHtml;
                 beforeEach(function(done) {