Skip to content

Commit 5dca925

Browse files
crisbetommalerba
authored andcommitted
fix(datepicker): don't revalidate if new date object for same date is passed through input (#20362)
If a function is used to provide a new date object for the `min` or `max` of a datepicker input, we currently trigger a revalidation on each change detection, because the input value is technically different. Historically this is something that we haven't fixed on our end, because it masks an anti-pattern on the user's end, however in this case it breaks in a non-obvious way and detecting it would be more code than just handling it. These changes add checks in several places so that we don't trigger validators or extra change detections if a new date object for the same date is provided. These changes also clean up the date range input which had both a `stateChanges` and `_stateChanges` streams which are required by two separate interfaces. Now there's a single `stateChanges`. Fixes #19907. (cherry picked from commit 6296da2)
1 parent a307cf1 commit 5dca925

File tree

8 files changed

+307
-40
lines changed

8 files changed

+307
-40
lines changed

src/material/datepicker/date-range-input.spec.ts

Lines changed: 119 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
1-
import {Type, Component, ViewChild, ElementRef} from '@angular/core';
1+
import {Type, Component, ViewChild, ElementRef, Directive} from '@angular/core';
22
import {ComponentFixture, TestBed, inject, fakeAsync, tick} from '@angular/core/testing';
3-
import {FormsModule, ReactiveFormsModule, FormGroup, FormControl} from '@angular/forms';
3+
import {
4+
FormsModule,
5+
ReactiveFormsModule,
6+
FormGroup,
7+
FormControl,
8+
NG_VALIDATORS,
9+
Validator,
10+
} from '@angular/forms';
411
import {NoopAnimationsModule} from '@angular/platform-browser/animations';
512
import {OverlayContainer} from '@angular/cdk/overlay';
613
import {MatNativeDateModule} from '@angular/material/core';
@@ -15,7 +22,9 @@ import {MatDateRangePicker} from './date-range-picker';
1522
import {MatStartDate, MatEndDate} from './date-range-input-parts';
1623

1724
describe('MatDateRangeInput', () => {
18-
function createComponent<T>(component: Type<T>): ComponentFixture<T> {
25+
function createComponent<T>(
26+
component: Type<T>,
27+
declarations: Type<any>[] = []): ComponentFixture<T> {
1928
TestBed.configureTestingModule({
2029
imports: [
2130
FormsModule,
@@ -26,7 +35,7 @@ describe('MatDateRangeInput', () => {
2635
ReactiveFormsModule,
2736
MatNativeDateModule,
2837
],
29-
declarations: [component],
38+
declarations: [component, ...declarations],
3039
});
3140

3241
return TestBed.createComponent(component);
@@ -626,6 +635,78 @@ describe('MatDateRangeInput', () => {
626635
endSubscription.unsubscribe();
627636
});
628637

638+
it('should not trigger validators if new date object for same date is set for `min`', () => {
639+
const fixture = createComponent(RangePickerWithCustomValidator, [CustomValidator]);
640+
fixture.detectChanges();
641+
const minDate = new Date(2019, 0, 1);
642+
const validator = fixture.componentInstance.validator;
643+
644+
validator.validate.calls.reset();
645+
fixture.componentInstance.min = minDate;
646+
fixture.detectChanges();
647+
expect(validator.validate).toHaveBeenCalledTimes(1);
648+
649+
fixture.componentInstance.min = new Date(minDate);
650+
fixture.detectChanges();
651+
652+
expect(validator.validate).toHaveBeenCalledTimes(1);
653+
});
654+
655+
it('should not trigger validators if new date object for same date is set for `max`', () => {
656+
const fixture = createComponent(RangePickerWithCustomValidator, [CustomValidator]);
657+
fixture.detectChanges();
658+
const maxDate = new Date(2120, 0, 1);
659+
const validator = fixture.componentInstance.validator;
660+
661+
validator.validate.calls.reset();
662+
fixture.componentInstance.max = maxDate;
663+
fixture.detectChanges();
664+
expect(validator.validate).toHaveBeenCalledTimes(1);
665+
666+
fixture.componentInstance.max = new Date(maxDate);
667+
fixture.detectChanges();
668+
669+
expect(validator.validate).toHaveBeenCalledTimes(1);
670+
});
671+
672+
it('should not emit to `stateChanges` if new date object for same date is set for `min`', () => {
673+
const fixture = createComponent(StandardRangePicker);
674+
fixture.detectChanges();
675+
676+
const minDate = new Date(2019, 0, 1);
677+
const spy = jasmine.createSpy('stateChanges spy');
678+
const subscription = fixture.componentInstance.rangeInput.stateChanges.subscribe(spy);
679+
680+
fixture.componentInstance.minDate = minDate;
681+
fixture.detectChanges();
682+
expect(spy).toHaveBeenCalledTimes(1);
683+
684+
fixture.componentInstance.minDate = new Date(minDate);
685+
fixture.detectChanges();
686+
expect(spy).toHaveBeenCalledTimes(1);
687+
688+
subscription.unsubscribe();
689+
});
690+
691+
it('should not emit to `stateChanges` if new date object for same date is set for `max`', () => {
692+
const fixture = createComponent(StandardRangePicker);
693+
fixture.detectChanges();
694+
695+
const maxDate = new Date(2120, 0, 1);
696+
const spy = jasmine.createSpy('stateChanges spy');
697+
const subscription = fixture.componentInstance.rangeInput.stateChanges.subscribe(spy);
698+
699+
fixture.componentInstance.maxDate = maxDate;
700+
fixture.detectChanges();
701+
expect(spy).toHaveBeenCalledTimes(1);
702+
703+
fixture.componentInstance.maxDate = new Date(maxDate);
704+
fixture.detectChanges();
705+
expect(spy).toHaveBeenCalledTimes(1);
706+
707+
subscription.unsubscribe();
708+
});
709+
629710
});
630711

631712
@Component({
@@ -736,3 +817,37 @@ class RangePickerNoLabel {
736817
@ViewChild('start') start: ElementRef<HTMLInputElement>;
737818
@ViewChild('end') end: ElementRef<HTMLInputElement>;
738819
}
820+
821+
822+
@Directive({
823+
selector: '[customValidator]',
824+
providers: [{
825+
provide: NG_VALIDATORS,
826+
useExisting: CustomValidator,
827+
multi: true
828+
}]
829+
})
830+
class CustomValidator implements Validator {
831+
validate = jasmine.createSpy('validate spy').and.returnValue(null);
832+
}
833+
834+
835+
@Component({
836+
template: `
837+
<mat-form-field>
838+
<mat-date-range-input [rangePicker]="rangePicker" [min]="min" [max]="max">
839+
<input matStartDate [(ngModel)]="start" customValidator/>
840+
<input matEndDate [(ngModel)]="end" customValidator/>
841+
</mat-date-range-input>
842+
843+
<mat-date-range-picker #rangePicker></mat-date-range-picker>
844+
</mat-form-field>
845+
`
846+
})
847+
class RangePickerWithCustomValidator {
848+
@ViewChild(CustomValidator) validator: CustomValidator;
849+
start: Date | null = null;
850+
end: Date | null = null;
851+
min: Date;
852+
max: Date;
853+
}

src/material/datepicker/date-range-input.ts

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
ElementRef,
2121
Inject,
2222
OnChanges,
23+
SimpleChanges,
2324
} from '@angular/core';
2425
import {MatFormFieldControl, MatFormField, MAT_FORM_FIELD} from '@angular/material/form-field';
2526
import {ThemePalette, DateAdapter} from '@angular/material/core';
@@ -34,7 +35,7 @@ import {
3435
} from './date-range-input-parts';
3536
import {MatDatepickerControl} from './datepicker-base';
3637
import {createMissingDateImplError} from './datepicker-errors';
37-
import {DateFilterFn} from './datepicker-input-base';
38+
import {DateFilterFn, dateInputsHaveChanged} from './datepicker-input-base';
3839
import {MatDateRangePicker, MatDateRangePickerInput} from './date-range-picker';
3940
import {DateRange, MatDateSelectionModel} from './date-selection-model';
4041

@@ -72,9 +73,6 @@ export class MatDateRangeInput<D> implements MatFormFieldControl<DateRange<D>>,
7273
return this._model ? this._model.selection : null;
7374
}
7475

75-
/** Emits when the input's state has changed. */
76-
stateChanges = new Subject<void>();
77-
7876
/** Unique ID for the input. */
7977
id = `mat-date-range-input-${nextUniqueId++}`;
8078

@@ -133,17 +131,25 @@ export class MatDateRangeInput<D> implements MatFormFieldControl<DateRange<D>>,
133131
@Input()
134132
get min(): D | null { return this._min; }
135133
set min(value: D | null) {
136-
this._min = this._dateAdapter.getValidDateOrNull(this._dateAdapter.deserialize(value));
137-
this._revalidate();
134+
const validValue = this._dateAdapter.getValidDateOrNull(this._dateAdapter.deserialize(value));
135+
136+
if (!this._dateAdapter.sameDate(validValue, this._min)) {
137+
this._min = validValue;
138+
this._revalidate();
139+
}
138140
}
139141
private _min: D | null;
140142

141143
/** The maximum valid date. */
142144
@Input()
143145
get max(): D | null { return this._max; }
144146
set max(value: D | null) {
145-
this._max = this._dateAdapter.getValidDateOrNull(this._dateAdapter.deserialize(value));
146-
this._revalidate();
147+
const validValue = this._dateAdapter.getValidDateOrNull(this._dateAdapter.deserialize(value));
148+
149+
if (!this._dateAdapter.sameDate(validValue, this._max)) {
150+
this._max = validValue;
151+
this._revalidate();
152+
}
147153
}
148154
private _max: D | null;
149155

@@ -159,7 +165,7 @@ export class MatDateRangeInput<D> implements MatFormFieldControl<DateRange<D>>,
159165

160166
if (newValue !== this._groupDisabled) {
161167
this._groupDisabled = newValue;
162-
this._stateChanges.next(undefined);
168+
this.stateChanges.next(undefined);
163169
}
164170
}
165171
_groupDisabled = false;
@@ -205,8 +211,8 @@ export class MatDateRangeInput<D> implements MatFormFieldControl<DateRange<D>>,
205211
*/
206212
ngControl: NgControl | null;
207213

208-
/** Emits when the input's state changes. */
209-
_stateChanges = new Subject<void>();
214+
/** Emits when the input's state has changed. */
215+
stateChanges = new Subject<void>();
210216

211217
constructor(
212218
private _changeDetectorRef: ChangeDetectorRef,
@@ -263,17 +269,18 @@ export class MatDateRangeInput<D> implements MatFormFieldControl<DateRange<D>>,
263269
// We don't need to unsubscribe from this, because we
264270
// know that the input streams will be completed on destroy.
265271
merge(this._startInput.stateChanges, this._endInput.stateChanges).subscribe(() => {
266-
this._stateChanges.next(undefined);
272+
this.stateChanges.next(undefined);
267273
});
268274
}
269275

270-
ngOnChanges() {
271-
this._stateChanges.next(undefined);
276+
ngOnChanges(changes: SimpleChanges) {
277+
if (dateInputsHaveChanged(changes, this._dateAdapter)) {
278+
this.stateChanges.next(undefined);
279+
}
272280
}
273281

274282
ngOnDestroy() {
275283
this.stateChanges.complete();
276-
this._stateChanges.unsubscribe();
277284
}
278285

279286
/** Gets the date at which the calendar should start. */

src/material/datepicker/datepicker-base.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,7 @@ export interface MatDatepickerControl<D> {
232232
disabled: boolean;
233233
dateFilter: DateFilterFn<D>;
234234
getConnectedOverlayOrigin(): ElementRef;
235-
_stateChanges: Observable<void>;
235+
stateChanges: Observable<void>;
236236
}
237237

238238
/** Base class for a datepicker. */
@@ -442,7 +442,7 @@ export abstract class MatDatepickerBase<C extends MatDatepickerControl<D>, S,
442442
this._inputStateChanges.unsubscribe();
443443
this._datepickerInput = input;
444444
this._inputStateChanges =
445-
input._stateChanges.subscribe(() => this._stateChanges.next(undefined));
445+
input.stateChanges.subscribe(() => this._stateChanges.next(undefined));
446446
return this._model;
447447
}
448448

src/material/datepicker/datepicker-input-base.ts

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
Output,
2020
AfterViewInit,
2121
OnChanges,
22+
SimpleChanges,
2223
} from '@angular/core';
2324
import {
2425
AbstractControl,
@@ -97,7 +98,7 @@ export abstract class MatDatepickerInputBase<S, D = ExtractDateTypeFromSelection
9798

9899
if (this._disabled !== newValue) {
99100
this._disabled = newValue;
100-
this._stateChanges.next(undefined);
101+
this.stateChanges.next(undefined);
101102
}
102103

103104
// We need to null check the `blur` method, because it's undefined during SSR.
@@ -125,7 +126,7 @@ export abstract class MatDatepickerInputBase<S, D = ExtractDateTypeFromSelection
125126
_valueChange = new EventEmitter<D | null>();
126127

127128
/** Emits when the internal state has changed */
128-
_stateChanges = new Subject<void>();
129+
stateChanges = new Subject<void>();
129130

130131
_onTouched = () => {};
131132
_validatorOnChange = () => {};
@@ -270,15 +271,17 @@ export abstract class MatDatepickerInputBase<S, D = ExtractDateTypeFromSelection
270271
this._isInitialized = true;
271272
}
272273

273-
ngOnChanges() {
274-
this._stateChanges.next(undefined);
274+
ngOnChanges(changes: SimpleChanges) {
275+
if (dateInputsHaveChanged(changes, this._dateAdapter)) {
276+
this.stateChanges.next(undefined);
277+
}
275278
}
276279

277280
ngOnDestroy() {
278281
this._valueChangesSubscription.unsubscribe();
279282
this._localeSubscription.unsubscribe();
280283
this._valueChange.complete();
281-
this._stateChanges.complete();
284+
this.stateChanges.complete();
282285
}
283286

284287
/** @docs-private */
@@ -394,3 +397,27 @@ export abstract class MatDatepickerInputBase<S, D = ExtractDateTypeFromSelection
394397
static ngAcceptInputType_value: any;
395398
static ngAcceptInputType_disabled: BooleanInput;
396399
}
400+
401+
/**
402+
* Checks whether the `SimpleChanges` object from an `ngOnChanges`
403+
* callback has any changes, accounting for date objects.
404+
*/
405+
export function dateInputsHaveChanged(
406+
changes: SimpleChanges,
407+
adapter: DateAdapter<unknown>): boolean {
408+
const keys = Object.keys(changes);
409+
410+
for (let key of keys) {
411+
const {previousValue, currentValue} = changes[key];
412+
413+
if (adapter.isDateInstance(previousValue) && adapter.isDateInstance(currentValue)) {
414+
if (!adapter.sameDate(previousValue, currentValue)) {
415+
return true;
416+
}
417+
} else {
418+
return true;
419+
}
420+
}
421+
422+
return false;
423+
}

src/material/datepicker/datepicker-input.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,17 +87,25 @@ export class MatDatepickerInput<D> extends MatDatepickerInputBase<D | null, D>
8787
@Input()
8888
get min(): D | null { return this._min; }
8989
set min(value: D | null) {
90-
this._min = this._dateAdapter.getValidDateOrNull(this._dateAdapter.deserialize(value));
91-
this._validatorOnChange();
90+
const validValue = this._dateAdapter.getValidDateOrNull(this._dateAdapter.deserialize(value));
91+
92+
if (!this._dateAdapter.sameDate(validValue, this._min)) {
93+
this._min = validValue;
94+
this._validatorOnChange();
95+
}
9296
}
9397
private _min: D | null;
9498

9599
/** The maximum valid date. */
96100
@Input()
97101
get max(): D | null { return this._max; }
98102
set max(value: D | null) {
99-
this._max = this._dateAdapter.getValidDateOrNull(this._dateAdapter.deserialize(value));
100-
this._validatorOnChange();
103+
const validValue = this._dateAdapter.getValidDateOrNull(this._dateAdapter.deserialize(value));
104+
105+
if (!this._dateAdapter.sameDate(validValue, this._max)) {
106+
this._max = validValue;
107+
this._validatorOnChange();
108+
}
101109
}
102110
private _max: D | null;
103111

src/material/datepicker/datepicker-toggle.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ export class MatDatepickerToggle<D> implements AfterContentInit, OnChanges, OnDe
120120
private _watchStateChanges() {
121121
const datepickerStateChanged = this.datepicker ? this.datepicker._stateChanges : observableOf();
122122
const inputStateChanged = this.datepicker && this.datepicker._datepickerInput ?
123-
this.datepicker._datepickerInput._stateChanges : observableOf();
123+
this.datepicker._datepickerInput.stateChanges : observableOf();
124124
const datepickerToggled = this.datepicker ?
125125
merge(this.datepicker.openedStream, this.datepicker.closedStream) :
126126
observableOf();

0 commit comments

Comments
 (0)