fix: compute property values in willChange (#8430)

vaadin · Jan 2, 2025 · 28684ad · 28684ad
1 parent 9a742f8
commit 28684ad
Show file tree

Hide file tree

Showing 9 changed files with 36 additions and 68 deletions.
diff --git a/packages/avatar-group/src/vaadin-avatar-group-mixin.js b/packages/avatar-group/src/vaadin-avatar-group-mixin.js
@@ -138,7 +138,6 @@ export const AvatarGroupMixin = (superClass) =>
           type: Array,
           observer: '__overflowItemsChanged',
           computed: '__computeOverflowItems(items, __itemsInView, maxItemsVisible)',
-          sync: true,
         },
 
         /** @private */

diff --git a/packages/component-base/src/polylit-mixin.js b/packages/component-base/src/polylit-mixin.js
@@ -185,7 +185,7 @@ const PolylitMixinImplementation = (superclass) => {
           this[name] = this[observer.method](...props);
         };
 
-        this.getOrCreateMap('__complexObservers').set(assignComputedMethod, observer.observerProps);
+        this.getOrCreateMap('__computedObservers').set(assignComputedMethod, observer.observerProps);
       }
 
       if (!options.attribute) {
@@ -245,6 +245,13 @@ const PolylitMixinImplementation = (superclass) => {
     /** @protected */
     ready() {}
 
+    /** @protected */
+    willUpdate(props) {
+      if (this.constructor.__computedObservers) {
+        this.__runComplexObservers(props, this.constructor.__computedObservers);
+      }
+    }
+
     /** @protected */
     updated(props) {
       const wasReadyInvoked = this.__isReadyInvoked;

diff --git a/packages/component-base/test/polylit-mixin.test.js b/packages/component-base/test/polylit-mixin.test.js
@@ -925,16 +925,26 @@ describe('PolylitMixin', () => {
         __computeValue(loading) {
           return loading ? 1 : 0;
         }
+
+        render() {
+          return html`${this.value}`;
+        }
       },
     );
 
-    it('should compute value', () => {
+    beforeEach(() => {
       element = fixtureSync(`<${tag}></${tag}>`);
+    });
+
+    it('should compute value', () => {
       expect(element.value).to.equal(1);
     });
 
+    it('should render computed value', () => {
+      expect(element.shadowRoot.textContent).to.equal('1');
+    });
+
     it('should update computed value', async () => {
-      element = fixtureSync(`<${tag}></${tag}>`);
       element.loading = false;
       await element.updateComplete;
       expect(element.value).to.equal(0);

diff --git a/packages/date-picker/src/vaadin-date-picker-mixin.js b/packages/date-picker/src/vaadin-date-picker-mixin.js
@@ -326,7 +326,6 @@ export const DatePickerMixin = (subclass) =>
         _minDate: {
           type: Date,
           computed: '__computeMinOrMaxDate(min)',
-          sync: true,
         },
 
         /**
@@ -337,7 +336,6 @@ export const DatePickerMixin = (subclass) =>
         _maxDate: {
           type: Date,
           computed: '__computeMinOrMaxDate(max)',
-          sync: true,
         },
 
         /** @private */

diff --git a/packages/date-picker/src/vaadin-lit-month-calendar.js b/packages/date-picker/src/vaadin-lit-month-calendar.js
@@ -98,26 +98,6 @@ class MonthCalendar extends MonthCalendarMixin(ThemableMixin(PolylitMixin(LitEle
       </table>
     `;
   }
-
-  /** @protected */
-  willUpdate(props) {
-    // Calculate these properties in `willUpdate()` instead of marking
-    // them as `computed` to avoid extra update because of `sync: true`
-    if (props.has('month') || props.has('i18n')) {
-      this._days = this._getDays(this.month, this.i18n);
-      this._weeks = this._getWeeks(this._days);
-    }
-
-    if (props.has('month') || props.has('minDate') || props.has('maxDate')) {
-      this.disabled = this._isDisabled(this.month, this.minDate, this.maxDate);
-    }
-
-    if (props.has('showWeekNumbers') || props.has('i18n')) {
-      // Currently only supported for locales that start the week on Monday.
-      this._showWeekNumbers = this.showWeekNumbers && this.i18n && this.i18n.firstDayOfWeek === 1;
-      this.toggleAttribute('week-numbers', this._showWeekNumbers);
-    }
-  }
 }
 
 defineCustomElement(MonthCalendar);
diff --git a/packages/date-picker/src/vaadin-month-calendar-mixin.js b/packages/date-picker/src/vaadin-month-calendar-mixin.js
@@ -98,16 +98,19 @@ export const MonthCalendarMixin = (superClass) =>
         disabled: {
           type: Boolean,
           reflectToAttribute: true,
+          computed: '_isDisabled(month, minDate, maxDate)',
         },
 
         /** @protected */
         _days: {
           type: Array,
+          computed: '_getDays(month, i18n, minDate, maxDate, isDateDisabled)',
         },
 
         /** @protected */
         _weeks: {
           type: Array,
+          computed: '_getWeeks(_days)',
         },
 
         /** @private */
@@ -123,7 +126,7 @@ export const MonthCalendarMixin = (superClass) =>
     }
 
     static get observers() {
-      return ['__focusedDateChanged(focusedDate, _days)'];
+      return ['__focusedDateChanged(focusedDate, _days)', '_showWeekNumbersChanged(showWeekNumbers, i18n)'];
     }
 
     get focusableDateElement() {
@@ -328,4 +331,13 @@ export const MonthCalendarMixin = (superClass) =>
 
       return ariaLabel;
     }
+
+    /** @private */
+    _showWeekNumbersChanged(showWeekNumbers, i18n) {
+      if (showWeekNumbers && i18n && i18n.firstDayOfWeek === 1) {
+        this.setAttribute('week-numbers', '');
+      } else {
+        this.removeAttribute('week-numbers');
+      }
+    }
   };
diff --git a/packages/date-picker/src/vaadin-month-calendar.js b/packages/date-picker/src/vaadin-month-calendar.js
@@ -71,41 +71,6 @@ class MonthCalendar extends MonthCalendarMixin(ThemableMixin(PolymerElement)) {
     return 'vaadin-month-calendar';
   }
 
-  static get properties() {
-    return {
-      /** @protected */
-      _days: {
-        type: Array,
-        computed: '_getDays(month, i18n, minDate, maxDate, isDateDisabled)',
-      },
-
-      /** @protected */
-      _weeks: {
-        type: Array,
-        computed: '_getWeeks(_days)',
-      },
-
-      disabled: {
-        type: Boolean,
-        reflectToAttribute: true,
-        computed: '_isDisabled(month, minDate, maxDate)',
-      },
-    };
-  }
-
-  static get observers() {
-    return ['_showWeekNumbersChanged(showWeekNumbers, i18n)'];
-  }
-
-  /** @private */
-  _showWeekNumbersChanged(showWeekNumbers, i18n) {
-    if (showWeekNumbers && i18n && i18n.firstDayOfWeek === 1) {
-      this.setAttribute('week-numbers', '');
-    } else {
-      this.removeAttribute('week-numbers');
-    }
-  }
-
   /** @private */
   // eslint-disable-next-line @typescript-eslint/max-params
   __getDatePart(date, focusedDate, selectedDate, minDate, maxDate, isDateDisabled, enteredDate, hasFocus) {

diff --git a/packages/field-base/test/field-mixin.test.js b/packages/field-base/test/field-mixin.test.js
@@ -164,7 +164,7 @@ const runTests = (defineHelper, baseMixin) => {
         element.invalid = true;
         element.setAttribute('error-message', 'This field is required');
         await nextUpdate(element);
-        clock.tick(150);
+        clock.tick(200);
         expect(announceRegion.textContent).to.equal('This field is required');
         expect(announceRegion.getAttribute('aria-live')).to.equal('assertive');
       });
@@ -173,15 +173,15 @@ const runTests = (defineHelper, baseMixin) => {
         element.invalid = true;
         element.errorMessage = 'This field is required';
         await nextUpdate(element);
-        clock.tick(150);
+        clock.tick(200);
         expect(announceRegion.textContent).to.equal('This field is required');
         expect(announceRegion.getAttribute('aria-live')).to.equal('assertive');
       });
 
       it('should not announce error message when field is valid', async () => {
         element.errorMessage = 'This field is required';
         await nextUpdate(element);
-        clock.tick(150);
+        clock.tick(200);
         expect(announceRegion.textContent).to.equal('');
       });
     });

diff --git a/packages/grid/src/vaadin-grid-column-mixin.js b/packages/grid/src/vaadin-grid-column-mixin.js
@@ -228,7 +228,6 @@ export const ColumnBaseMixin = (superClass) =>
         _headerRenderer: {
           type: Function,
           computed: '_computeHeaderRenderer(headerRenderer, header, __initialized)',
-          sync: true,
         },
 
         /**
@@ -255,7 +254,6 @@ export const ColumnBaseMixin = (superClass) =>
         _footerRenderer: {
           type: Function,
           computed: '_computeFooterRenderer(footerRenderer, __initialized)',
-          sync: true,
         },
 
         /**
@@ -913,7 +911,6 @@ export const GridColumnMixin = (superClass) =>
         _renderer: {
           type: Function,
           computed: '_computeRenderer(renderer, __initialized)',
-          sync: true,
         },
 
         /**