fix: ensure ready() is called after observers (#8254)

vaadin · Dec 2, 2024 · d5e20c6 · d5e20c6
1 parent 642a050
commit d5e20c6
Show file tree

Hide file tree

Showing 7 changed files with 87 additions and 42 deletions.
diff --git a/packages/component-base/src/polylit-mixin.js b/packages/component-base/src/polylit-mixin.js
@@ -213,6 +213,9 @@ const PolylitMixinImplementation = (superclass) => {
 
     /** @protected */
     updated(props) {
+      const wasReadyInvoked = this.__isReadyInvoked;
+      this.__isReadyInvoked = true;
+
       if (this.constructor.__observers) {
         this.__runObservers(props, this.constructor.__observers);
       }
@@ -233,8 +236,7 @@ const PolylitMixinImplementation = (superclass) => {
         this.__runNotifyProps(props, this.constructor.__notifyProps);
       }
 
-      if (!this.__isReadyInvoked) {
-        this.__isReadyInvoked = true;
+      if (!wasReadyInvoked) {
         this.ready();
       }
     }

diff --git a/packages/component-base/test/polylit-mixin.test.js b/packages/component-base/test/polylit-mixin.test.js
@@ -1069,6 +1069,79 @@ describe('PolylitMixin', () => {
     });
   });
 
+  describe('sync observers', () => {
+    let element;
+    const readySpy = sinon.spy();
+    const openedChangedSpy = sinon.spy();
+    const headerChangedSpy = sinon.spy();
+    const contentChangedSpy = sinon.spy();
+
+    const tag = defineCE(
+      class extends PolylitMixin(LitElement) {
+        static get properties() {
+          return {
+            opened: {
+              type: Boolean,
+              sync: true,
+            },
+
+            header: {
+              type: String,
+              sync: true,
+            },
+
+            content: {
+              type: String,
+              sync: true,
+            },
+          };
+        }
+
+        static get observers() {
+          return ['openedChanged(opened)', 'headerChanged(opened, header)', 'contentChanged(opened, content)'];
+        }
+
+        ready() {
+          super.ready();
+          readySpy();
+        }
+
+        openedChanged(opened) {
+          openedChangedSpy();
+
+          if (opened) {
+            this.header = 'Header';
+            this.content = 'Content';
+          }
+        }
+
+        headerChanged(_opened, _header) {
+          headerChangedSpy();
+        }
+
+        contentChanged(_opened, _content) {
+          contentChangedSpy();
+        }
+      },
+    );
+
+    beforeEach(async () => {
+      element = fixtureSync(`<${tag} opened></${tag}>`);
+      await element.updateComplete;
+    });
+
+    it('should call ready after observers during initialization', () => {
+      expect(openedChangedSpy).to.be.calledOnce;
+      expect(headerChangedSpy).to.be.calledTwice;
+      expect(contentChangedSpy).to.be.calledTwice;
+
+      expect(readySpy).to.be.calledOnce;
+      expect(readySpy).to.be.calledAfter(openedChangedSpy);
+      expect(readySpy).to.be.calledAfter(headerChangedSpy);
+      expect(readySpy).to.be.calledAfter(contentChangedSpy);
+    });
+  });
+
   describe('setProperties()', () => {
     let element;
 

diff --git a/packages/date-picker/src/vaadin-lit-date-picker.js b/packages/date-picker/src/vaadin-lit-date-picker.js
@@ -109,8 +109,8 @@ class DatePicker extends DatePickerMixin(InputControlMixin(ThemableMixin(Element
   }
 
   /** @protected */
-  firstUpdated() {
-    super.firstUpdated();
+  ready() {
+    super.ready();
 
     this.addController(
       new InputController(this, (input) => {

diff --git a/packages/number-field/src/vaadin-lit-number-field.js b/packages/number-field/src/vaadin-lit-number-field.js
@@ -8,7 +8,6 @@ import { html, LitElement } from 'lit';
 import { defineCustomElement } from '@vaadin/component-base/src/define.js';
 import { ElementMixin } from '@vaadin/component-base/src/element-mixin.js';
 import { PolylitMixin } from '@vaadin/component-base/src/polylit-mixin.js';
-import { TooltipController } from '@vaadin/component-base/src/tooltip-controller.js';
 import { inputFieldShared } from '@vaadin/field-base/src/styles/input-field-shared-styles.js';
 import { ThemableMixin } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mixin.js';
 import { NumberFieldMixin } from './vaadin-number-field-mixin.js';
@@ -84,30 +83,6 @@ class NumberField extends NumberFieldMixin(ThemableMixin(ElementMixin(PolylitMix
       <slot name="tooltip"></slot>
     `;
   }
-
-  /** @protected */
-  ready() {
-    super.ready();
-
-    this._tooltipController = new TooltipController(this);
-    this.addController(this._tooltipController);
-    this._tooltipController.setPosition('top');
-    this._tooltipController.setAriaTarget(this.inputElement);
-  }
-
-  /**
-   * Override method from `InputConstraintsMixin`
-   * to create observer after the initial update
-   * and preserve invalid state set as attribute.
-   *
-   * @protected
-   * @override
-   */
-  async _createConstraintsObserver() {
-    await this.updateComplete;
-
-    super._createConstraintsObserver();
-  }
 }
 
 defineCustomElement(NumberField);

diff --git a/packages/number-field/src/vaadin-number-field-mixin.js b/packages/number-field/src/vaadin-number-field-mixin.js
@@ -4,6 +4,7 @@
  * This program is available under Apache License Version 2.0, available at https://vaadin.com/license/
  */
 import { getDeepActiveElement } from '@vaadin/a11y-base/src/focus-utils.js';
+import { TooltipController } from '@vaadin/component-base/src/tooltip-controller';
 import { InputController } from '@vaadin/field-base/src/input-controller.js';
 import { InputFieldMixin } from '@vaadin/field-base/src/input-field-mixin.js';
 import { LabelledInputController } from '@vaadin/field-base/src/labelled-input-controller.js';
@@ -127,6 +128,11 @@ export const NumberFieldMixin = (superClass) =>
       );
 
       this.addController(new LabelledInputController(this.inputElement, this._labelController));
+
+      this._tooltipController = new TooltipController(this);
+      this.addController(this._tooltipController);
+      this._tooltipController.setPosition('top');
+      this._tooltipController.setAriaTarget(this.inputElement);
     }
 
     /**

diff --git a/packages/number-field/src/vaadin-number-field.js b/packages/number-field/src/vaadin-number-field.js
@@ -7,7 +7,6 @@ import '@vaadin/input-container/src/vaadin-input-container.js';
 import { html, PolymerElement } from '@polymer/polymer';
 import { defineCustomElement } from '@vaadin/component-base/src/define.js';
 import { ElementMixin } from '@vaadin/component-base/src/element-mixin.js';
-import { TooltipController } from '@vaadin/component-base/src/tooltip-controller.js';
 import { inputFieldShared } from '@vaadin/field-base/src/styles/input-field-shared-styles.js';
 import { registerStyles, ThemableMixin } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mixin.js';
 import { NumberFieldMixin } from './vaadin-number-field-mixin.js';
@@ -128,16 +127,6 @@ export class NumberField extends NumberFieldMixin(ThemableMixin(ElementMixin(Pol
       <slot name="tooltip"></slot>
     `;
   }
-
-  /** @protected */
-  ready() {
-    super.ready();
-
-    this._tooltipController = new TooltipController(this);
-    this.addController(this._tooltipController);
-    this._tooltipController.setPosition('top');
-    this._tooltipController.setAriaTarget(this.inputElement);
-  }
 }
 
 defineCustomElement(NumberField);
diff --git a/packages/time-picker/src/vaadin-lit-time-picker.js b/packages/time-picker/src/vaadin-lit-time-picker.js
@@ -115,8 +115,8 @@ class TimePicker extends TimePickerMixin(ThemableMixin(ElementMixin(PolylitMixin
   }
 
   /** @protected */
-  firstUpdated() {
-    super.firstUpdated();
+  ready() {
+    super.ready();
 
     this.addController(
       new InputController(