Skip to content

Commit

Permalink
fix(datepicker): validate that input actually parses (#5711)
Browse files Browse the repository at this point in the history
* fix(datepicker): throw when value is set to invalid type

BREAKING CHANGE: You must now use an actual Date object rather than a
string when setting the value of the datepicker programmatically
(through value, ngModel, or formControl).

* addressed comments

* validate that datepicker input parses

* tweak handling of invalid dates.

* fix tests

* address feedback

* make sure datepicker selected date is valid

* rework validation again

* addressed comments

* fix lint
  • Loading branch information
mmalerba authored and tinayuangao committed Aug 1, 2017
1 parent 0d80a77 commit 8bb54ca
Show file tree
Hide file tree
Showing 8 changed files with 125 additions and 32 deletions.
3 changes: 3 additions & 0 deletions src/demo-app/datepicker/datepicker-demo.html
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ <h2>Result</h2>
placeholder="Pick a date"
(dateInput)="onDateInput($event)"
(dateChange)="onDateChange($event)">
<md-error *ngIf="resultPickerModel.hasError('mdDatepickerParse')">
"{{resultPickerModel.getError('mdDatepickerParse').text}}" is not a valid date!
</md-error>
<md-error *ngIf="resultPickerModel.hasError('mdDatepickerMin')">Too early!</md-error>
<md-error *ngIf="resultPickerModel.hasError('mdDatepickerMax')">Too late!</md-error>
<md-error *ngIf="resultPickerModel.hasError('mdDatepickerFilter')">Date unavailable!</md-error>
Expand Down
20 changes: 17 additions & 3 deletions src/lib/core/datetime/date-adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,15 +107,15 @@ export abstract class DateAdapter<D> {
* @param value The value to parse.
* @param parseFormat The expected format of the value being parsed
* (type is implementation-dependent).
* @returns The parsed date, or null if date could not be parsed.
* @returns The parsed date.
*/
abstract parse(value: any, parseFormat: any): D | null;

/**
* Formats a date as a string.
* @param date The value to parse.
* @param date The value to format.
* @param displayFormat The format to use to display the date as a string.
* @returns The parsed date, or null if date could not be parsed.
* @returns The formatted date string.
*/
abstract format(date: D, displayFormat: any): string;

Expand Down Expand Up @@ -156,6 +156,20 @@ export abstract class DateAdapter<D> {
*/
abstract getISODateString(date: D): string;

/**
* Checks whether the given object is considered a date instance by this DateAdapter.
* @param obj The object to check
* @returns Whether the object is a date instance.
*/
abstract isDateInstance(obj: any): boolean;

/**
* Checks whether the given date is valid.
* @param date The date to check.
* @returns Whether the date is valid.
*/
abstract isValid(date: D): boolean;

/**
* Sets the locale used for all dates.
* @param locale The new locale.
Expand Down
31 changes: 29 additions & 2 deletions src/lib/core/datetime/native-date-adapter.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,13 @@ describe('NativeDateAdapter', () => {
expect(adapter.parse(date)).not.toBe(date);
});

it('should parse invalid value as null', () => {
expect(adapter.parse('hello')).toBeNull();
it('should parse invalid value as invalid', () => {
let d = adapter.parse('hello');
expect(d).not.toBeNull();
expect(adapter.isDateInstance(d))
.toBe(true, 'Expected string to have been fed through Date.parse');
expect(adapter.isValid(d as Date))
.toBe(false, 'Expected to parse as "invalid date" object');
});

it('should format', () => {
Expand Down Expand Up @@ -238,6 +243,11 @@ describe('NativeDateAdapter', () => {
}
});

it('should throw when attempting to format invalid date', () => {
expect(() => adapter.format(new Date(NaN), {}))
.toThrowError(/NativeDateAdapter: Cannot format invalid date\./);
});

it('should add years', () => {
expect(adapter.addCalendarYears(new Date(2017, JAN, 1), 1)).toEqual(new Date(2018, JAN, 1));
expect(adapter.addCalendarYears(new Date(2017, JAN, 1), -1)).toEqual(new Date(2016, JAN, 1));
Expand Down Expand Up @@ -304,6 +314,23 @@ describe('NativeDateAdapter', () => {
expect(adapter.format(new Date(1800, 7, 14), {day: 'numeric'})).toBe('Thu Aug 14 1800');
}
});

it('should count today as a valid date instance', () => {
let d = new Date();
expect(adapter.isValid(d)).toBe(true);
expect(adapter.isDateInstance(d)).toBe(true);
});

it('should count an invalid date as an invalid date instance', () => {
let d = new Date(NaN);
expect(adapter.isValid(d)).toBe(false);
expect(adapter.isDateInstance(d)).toBe(true);
});

it('should count a string as not a date instance', () => {
let d = '1/1/2017';
expect(adapter.isDateInstance(d)).toBe(false);
});
});


Expand Down
17 changes: 15 additions & 2 deletions src/lib/core/datetime/native-date-adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,11 +157,16 @@ export class NativeDateAdapter extends DateAdapter<Date> {
parse(value: any): Date | null {
// We have no way using the native JS Date to set the parse format or locale, so we ignore these
// parameters.
let timestamp = typeof value == 'number' ? value : Date.parse(value);
return isNaN(timestamp) ? null : new Date(timestamp);
if (typeof value == 'number') {
return new Date(value);
}
return value ? new Date(Date.parse(value)) : null;
}

format(date: Date, displayFormat: Object): string {
if (!this.isValid(date)) {
throw Error('NativeDateAdapter: Cannot format invalid date.');
}
if (SUPPORTS_INTL_API) {
if (this.useUtcForDisplay) {
date = new Date(Date.UTC(
Expand Down Expand Up @@ -207,6 +212,14 @@ export class NativeDateAdapter extends DateAdapter<Date> {
].join('-');
}

isDateInstance(obj: any) {
return obj instanceof Date;
}

isValid(date: Date) {
return !isNaN(date.getTime());
}

/** Creates a date but allows the month and date to overflow. */
private _createDateWithOverflow(year: number, month: number, date: number) {
let result = new Date(year, month, date);
Expand Down
6 changes: 3 additions & 3 deletions src/lib/datepicker/calendar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,13 @@ export class MdCalendar<D> implements AfterContentInit, OnDestroy {
@Input() startView: 'month' | 'year' = 'month';

/** The currently selected date. */
@Input() selected: D;
@Input() selected: D | null;

/** The minimum selectable date. */
@Input() minDate: D;
@Input() minDate: D | null;

/** The maximum selectable date. */
@Input() maxDate: D;
@Input() maxDate: D | null;

/** A function used to filter which dates are selectable. */
@Input() dateFilter: (date: D) => boolean;
Expand Down
51 changes: 38 additions & 13 deletions src/lib/datepicker/datepicker-input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,36 +112,41 @@ export class MdDatepickerInput<D> implements AfterContentInit, ControlValueAcces
/** The value of the input. */
@Input()
get value(): D | null {
return this._dateAdapter.parse(this._elementRef.nativeElement.value,
this._dateFormats.parse.dateInput);
return this._getValidDateOrNull(this._dateAdapter.parse(
this._elementRef.nativeElement.value, this._dateFormats.parse.dateInput));
}
set value(value: D | null) {
let date = this._dateAdapter.parse(value, this._dateFormats.parse.dateInput);
if (value != null && !this._dateAdapter.isDateInstance(value)) {
throw Error('Datepicker: value not recognized as a date object by DateAdapter.');
}
this._lastValueValid = !value || this._dateAdapter.isValid(value);
value = this._getValidDateOrNull(value);

let oldDate = this.value;
this._renderer.setProperty(this._elementRef.nativeElement, 'value',
date ? this._dateAdapter.format(date, this._dateFormats.display.dateInput) : '');
if (!this._dateAdapter.sameDate(oldDate, date)) {
this._valueChange.emit(date);
value ? this._dateAdapter.format(value, this._dateFormats.display.dateInput) : '');
if (!this._dateAdapter.sameDate(oldDate, value)) {
this._valueChange.emit(value);
}
}

/** The minimum valid date. */
@Input()
get min(): D { return this._min; }
set min(value: D) {
get min(): D | null { return this._min; }
set min(value: D | null) {
this._min = value;
this._validatorOnChange();
}
private _min: D;
private _min: D | null;

/** The maximum valid date. */
@Input()
get max(): D { return this._max; }
set max(value: D) {
get max(): D | null { return this._max; }
set max(value: D | null) {
this._max = value;
this._validatorOnChange();
}
private _max: D;
private _max: D | null;

/** Whether the datepicker-input is disabled. */
@Input()
Expand All @@ -168,6 +173,12 @@ export class MdDatepickerInput<D> implements AfterContentInit, ControlValueAcces

private _datepickerSubscription: Subscription;

/** The form control validator for whether the input parses. */
private _parseValidator: ValidatorFn = (): ValidationErrors | null => {
return this._lastValueValid ?
null : {'mdDatepickerParse': {'text': this._elementRef.nativeElement.value}};
}

/** The form control validator for the min date. */
private _minValidator: ValidatorFn = (control: AbstractControl): ValidationErrors | null => {
return (!this.min || !control.value ||
Expand All @@ -190,7 +201,11 @@ export class MdDatepickerInput<D> implements AfterContentInit, ControlValueAcces

/** The combined form control validator for this input. */
private _validator: ValidatorFn | null =
Validators.compose([this._minValidator, this._maxValidator, this._filterValidator]);
Validators.compose(
[this._parseValidator, this._minValidator, this._maxValidator, this._filterValidator]);

/** Whether the last value set on the input was valid. */
private _lastValueValid = false;

constructor(
private _elementRef: ElementRef,
Expand Down Expand Up @@ -270,6 +285,8 @@ export class MdDatepickerInput<D> implements AfterContentInit, ControlValueAcces

_onInput(value: string) {
let date = this._dateAdapter.parse(value, this._dateFormats.parse.dateInput);
this._lastValueValid = !date || this._dateAdapter.isValid(date);
date = this._getValidDateOrNull(date);
this._cvaOnChange(date);
this._valueChange.emit(date);
this.dateInput.emit(new MdDatepickerInputEvent(this, this._elementRef.nativeElement));
Expand All @@ -278,4 +295,12 @@ export class MdDatepickerInput<D> implements AfterContentInit, ControlValueAcces
_onChange() {
this.dateChange.emit(new MdDatepickerInputEvent(this, this._elementRef.nativeElement));
}

/**
* @param obj The object to check.
* @returns The given object if it is both a date instance and valid, otherwise null.
*/
private _getValidDateOrNull(obj: any): D | null {
return (this._dateAdapter.isDateInstance(obj) && this._dateAdapter.isValid(obj)) ? obj : null;
}
}
13 changes: 11 additions & 2 deletions src/lib/datepicker/datepicker.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,15 @@ describe('MdDatepicker', () => {
expect(ownedElement).not.toBeNull();
expect((ownedElement as Element).tagName.toLowerCase()).toBe('md-calendar');
});

it('should throw when given wrong data type', () => {
testComponent.date = '1/1/2017' as any;

expect(() => fixture.detectChanges())
.toThrowError(/Datepicker: value not recognized as a date object by DateAdapter\./);

testComponent.date = null;
});
});

describe('datepicker with too many inputs', () => {
Expand Down Expand Up @@ -902,7 +911,7 @@ describe('MdDatepicker', () => {
class StandardDatepicker {
touch = false;
disabled = false;
date = new Date(2020, JAN, 1);
date: Date | null = new Date(2020, JAN, 1);
@ViewChild('d') datepicker: MdDatepicker<Date>;
@ViewChild(MdDatepickerInput) datepickerInput: MdDatepickerInput<Date>;
}
Expand Down Expand Up @@ -1008,7 +1017,7 @@ class InputContainerDatepicker {
})
class DatepickerWithMinAndMaxValidation {
@ViewChild('d') datepicker: MdDatepicker<Date>;
date: Date;
date: Date | null;
minDate = new Date(2010, JAN, 1);
maxDate = new Date(2020, JAN, 1);
}
Expand Down
16 changes: 9 additions & 7 deletions src/lib/datepicker/datepicker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,13 +124,13 @@ export class MdDatepickerContent<D> implements AfterContentInit {
export class MdDatepicker<D> implements OnDestroy {
/** The date to open the calendar to initially. */
@Input()
get startAt(): D {
get startAt(): D | null {
// If an explicit startAt is set we start there, otherwise we start at whatever the currently
// selected value is.
return this._startAt || (this._datepickerInput ? this._datepickerInput.value : null);
}
set startAt(date: D) { this._startAt = date; }
private _startAt: D;
set startAt(date: D | null) { this._startAt = date; }
private _startAt: D | null;

/** The view that the calendar should start in. */
@Input() startView: 'month' | 'year' = 'month';
Expand Down Expand Up @@ -164,15 +164,17 @@ export class MdDatepicker<D> implements OnDestroy {
id = `md-datepicker-${datepickerUid++}`;

/** The currently selected date. */
_selected: D | null = null;
get _selected(): D | null { return this._validSelected; }
set _selected(value: D | null) { this._validSelected = value; }
private _validSelected: D | null = null;

/** The minimum selectable date. */
get _minDate(): D {
get _minDate(): D | null {
return this._datepickerInput && this._datepickerInput.min;
}

/** The maximum selectable date. */
get _maxDate(): D {
get _maxDate(): D | null {
return this._datepickerInput && this._datepickerInput.max;
}

Expand Down Expand Up @@ -240,7 +242,7 @@ export class MdDatepicker<D> implements OnDestroy {
}
this._datepickerInput = input;
this._inputSubscription =
this._datepickerInput._valueChange.subscribe((value: D) => this._selected = value);
this._datepickerInput._valueChange.subscribe((value: D | null) => this._selected = value);
}

/** Open the calendar. */
Expand Down

0 comments on commit 8bb54ca

Please sign in to comment.