fix: don't throw when renderer is set before adding to DOM (#379)

vaadin · May 12, 2021 · 87145e5 · 87145e5
1 parent 8542c7d
commit 87145e5
Show file tree

Hide file tree

Showing 2 changed files with 122 additions and 93 deletions.
diff --git a/packages/vaadin-notification/src/vaadin-notification.js b/packages/vaadin-notification/src/vaadin-notification.js
@@ -302,7 +302,7 @@ class NotificationElement extends ThemePropertyMixin(ElementMixin(PolymerElement
   }
 
   static get observers() {
-    return ['_durationChanged(duration, opened)', '_rendererChanged(renderer, opened)'];
+    return ['_durationChanged(duration, opened)', '_rendererChanged(renderer, opened, _card)'];
   }
 
   /** @protected */
@@ -326,12 +326,16 @@ class NotificationElement extends ThemePropertyMixin(ElementMixin(PolymerElement
   }
 
   /** @private */
-  _rendererChanged(renderer, opened) {
+  _rendererChanged(renderer, opened, card) {
+    if (!card) {
+      return;
+    }
+
     const rendererChanged = this._oldRenderer !== renderer;
     this._oldRenderer = renderer;
 
     if (rendererChanged) {
-      this._card.innerHTML = '';
+      card.innerHTML = '';
     }
 
     if (opened) {

diff --git a/packages/vaadin-notification/test/renderer.test.js b/packages/vaadin-notification/test/renderer.test.js
@@ -4,107 +4,132 @@ import sinon from 'sinon';
 import '@vaadin/vaadin-template-renderer';
 import '../vaadin-notification.js';
 
-describe('vaadin-notification', () => {
-  let notification;
-  let rendererContent;
-
-  beforeEach(() => {
-    rendererContent = document.createElement('p');
-    rendererContent.textContent = 'renderer-content';
-
-    notification = fixtureSync('<vaadin-notification></vaadin-notification>');
-
-    // Force sync card attaching and removal instead of waiting for the animation
-    sinon.stub(notification, '_animatedAppendNotificationCard').callsFake(() => notification._appendNotificationCard());
-    sinon.stub(notification, '_animatedRemoveNotificationCard').callsFake(() => notification._removeNotificationCard());
-
-    notification.open();
-  });
-
-  afterEach(() => {
-    // Close to stop all pending timers.
-    notification.close();
-    // delete singleton reference, so as it's created in next test
-    delete notification.constructor._container;
-  });
-
-  it('should use renderer when it is defined', () => {
-    notification.renderer = (root) => {
-      root.appendChild(rendererContent);
-    };
-    notification.opened = true;
-    expect(notification._card.textContent.trim()).to.equal('renderer-content');
-  });
-
-  it('renderer should receive notification when defined', () => {
-    notification.renderer = (root, notification) => {
-      expect(notification).to.eql(notification);
-    };
-  });
-
-  it('should remove template when added after renderer', () => {
-    notification.renderer = () => {};
-    const template = document.createElement('template');
-    expect(() => {
-      notification.appendChild(template);
-      notification._observer.flush();
-    }).to.throw(Error);
-    expect(notification._notificationTemplate).to.be.not.ok;
-  });
-
-  it('should be possible to manually invoke renderer', () => {
-    notification.renderer = sinon.spy();
-    notification.opened = true;
-    expect(notification.renderer.calledOnce).to.be.true;
-    notification.render();
-    expect(notification.renderer.calledTwice).to.be.true;
-  });
+describe('renderer', () => {
+  describe('basic', () => {
+    let notification;
+    let rendererContent;
+
+    beforeEach(() => {
+      rendererContent = document.createElement('p');
+      rendererContent.textContent = 'renderer-content';
+
+      notification = fixtureSync('<vaadin-notification></vaadin-notification>');
+
+      // Force sync card attaching and removal instead of waiting for the animation
+      sinon
+        .stub(notification, '_animatedAppendNotificationCard')
+        .callsFake(() => notification._appendNotificationCard());
+      sinon
+        .stub(notification, '_animatedRemoveNotificationCard')
+        .callsFake(() => notification._removeNotificationCard());
+
+      notification.open();
+    });
+
+    afterEach(() => {
+      // Close to stop all pending timers.
+      notification.close();
+      // delete singleton reference, so as it's created in next test
+      delete notification.constructor._container;
+    });
+
+    it('should use renderer when it is defined', () => {
+      notification.renderer = (root) => {
+        root.appendChild(rendererContent);
+      };
+      notification.opened = true;
+      expect(notification._card.textContent.trim()).to.equal('renderer-content');
+    });
 
-  it('should provide root from the previous renderer call', () => {
-    notification.renderer = (root) => {
-      const generatedContent = document.createTextNode('rendered');
-      root.appendChild(generatedContent);
-    };
-    notification.opened = true;
-    notification.opened = false;
-    notification.opened = true;
-    expect(notification._card.textContent.trim()).to.equal('renderedrendered');
-  });
+    it('renderer should receive notification when defined', () => {
+      notification.renderer = (root, notification) => {
+        expect(notification).to.eql(notification);
+      };
+    });
+
+    it('should remove template when added after renderer', () => {
+      notification.renderer = () => {};
+      const template = document.createElement('template');
+      expect(() => {
+        notification.appendChild(template);
+        notification._observer.flush();
+      }).to.throw(Error);
+      expect(notification._notificationTemplate).to.be.not.ok;
+    });
+
+    it('should be possible to manually invoke renderer', () => {
+      notification.renderer = sinon.spy();
+      notification.opened = true;
+      expect(notification.renderer.calledOnce).to.be.true;
+      notification.render();
+      expect(notification.renderer.calledTwice).to.be.true;
+    });
 
-  it('should clear the root when renderer changed', () => {
-    for (let i = 0; i < 2; i++) {
+    it('should provide root from the previous renderer call', () => {
       notification.renderer = (root) => {
-        const generatedContent = document.createTextNode('rendered-' + i);
+        const generatedContent = document.createTextNode('rendered');
         root.appendChild(generatedContent);
       };
       notification.opened = true;
-      expect(notification._card.textContent.trim()).to.equal('rendered-' + i);
-    }
-  });
+      notification.opened = false;
+      notification.opened = true;
+      expect(notification._card.textContent.trim()).to.equal('renderedrendered');
+    });
+
+    it('should clear the root when renderer changed', () => {
+      for (let i = 0; i < 2; i++) {
+        notification.renderer = (root) => {
+          const generatedContent = document.createTextNode('rendered-' + i);
+          root.appendChild(generatedContent);
+        };
+        notification.opened = true;
+        expect(notification._card.textContent.trim()).to.equal('rendered-' + i);
+      }
+    });
+
+    it('should open notification when renderer is defined after notification opened', () => {
+      notification.opened = true;
+      notification.renderer = (root) => {
+        root.appendChild(rendererContent);
+      };
+      const clientRect = notification._card.getBoundingClientRect();
+      expect(clientRect.x).to.not.equal(0);
+      expect(clientRect.y).to.not.equal(0);
+      expect(clientRect.width).to.not.equal(0);
+      expect(clientRect.height).to.not.equal(0);
+      expect(clientRect.top).to.not.equal(0);
+    });
+
+    it('should clear the notification card when removing the renderer', () => {
+      notification.opened = true;
+      notification.renderer = (root) => {
+        root.innerHTML = 'foo';
+      };
+
+      expect(notification._card.textContent).to.equal('foo');
+
+      notification.renderer = null;
 
-  it('should open notification when renderer is defined after notification opened', () => {
-    notification.opened = true;
-    notification.renderer = (root) => {
-      root.appendChild(rendererContent);
-    };
-    const clientRect = notification._card.getBoundingClientRect();
-    expect(clientRect.x).to.not.equal(0);
-    expect(clientRect.y).to.not.equal(0);
-    expect(clientRect.width).to.not.equal(0);
-    expect(clientRect.height).to.not.equal(0);
-    expect(clientRect.top).to.not.equal(0);
+      expect(notification._card.textContent).to.equal('');
+    });
   });
 
-  it('should clear the notification card when removing the renderer', () => {
-    notification.opened = true;
-    notification.renderer = (root) => {
-      root.innerHTML = 'foo';
-    };
+  describe('set before connected', () => {
+    let notification;
 
-    expect(notification._card.textContent).to.equal('foo');
+    beforeEach(() => {
+      notification = document.createElement('vaadin-notification');
+    });
 
-    notification.renderer = null;
+    afterEach(() => {
+      document.body.removeChild(notification);
+    });
 
-    expect(notification._card.textContent).to.equal('');
+    it('should not throw when the renderer is set before adding to DOM', () => {
+      expect(() => {
+        notification.renderer = () => {};
+        document.body.appendChild(notification);
+      }).to.not.throw(Error);
+    });
   });
 });