From 083fab0e2b683508834bc9a8dbc65e8ddde257dd Mon Sep 17 00:00:00 2001 From: Miles Malerba Date: Tue, 26 Sep 2017 16:08:13 -0700 Subject: [PATCH] fix(datepicker): allow `DateAdapter` authors to have more control over what can/can't be coerced to a date BREAKING CHANGES: - `fromIso8601` method on `DateAdapter` removed in favor of `coerceToDate` --- src/lib/core/datetime/date-adapter.ts | 10 ++++--- .../core/datetime/native-date-adapter.spec.ts | 29 ++++++++++++------ src/lib/core/datetime/native-date-adapter.ts | 28 +++++++++++------ .../datepicker/coerce-date-property.spec.ts | 9 +++--- src/lib/datepicker/coerce-date-property.ts | 16 ++++------ .../adapter/moment-date-adapter.spec.ts | 30 +++++++++++++------ .../adapter/moment-date-adapter.ts | 25 ++++++++++++---- 7 files changed, 95 insertions(+), 52 deletions(-) diff --git a/src/lib/core/datetime/date-adapter.ts b/src/lib/core/datetime/date-adapter.ts index ec645c214882..e89f5b1e94bb 100644 --- a/src/lib/core/datetime/date-adapter.ts +++ b/src/lib/core/datetime/date-adapter.ts @@ -171,11 +171,13 @@ export abstract class DateAdapter { abstract toIso8601(date: D): string; /** - * Creates a date from an RFC 3339 compatible string (https://tools.ietf.org/html/rfc3339). - * @param iso8601String The ISO date string to create a date from - * @returns The date created from the ISO date string. + * Attempts to coerce a value to a date object (e.g. a ISO 8601 string). + * @param value The value to be coerced to a date object. + * @returns The coerced date object, either a valid date, null if the value can be coerced to a + * null date (e.g. the empty string), or an invalid date to indicate that the value could not + * be coerced. */ - abstract fromIso8601(iso8601String: string): D | null; + abstract coerceToDate(value: any): D | null; /** * Checks whether the given object is considered a date instance by this DateAdapter. diff --git a/src/lib/core/datetime/native-date-adapter.spec.ts b/src/lib/core/datetime/native-date-adapter.spec.ts index 92371ee0742f..472d156466eb 100644 --- a/src/lib/core/datetime/native-date-adapter.spec.ts +++ b/src/lib/core/datetime/native-date-adapter.spec.ts @@ -6,6 +6,17 @@ import {DateAdapter, MAT_DATE_LOCALE, NativeDateAdapter, NativeDateModule} from const SUPPORTS_INTL = typeof Intl != 'undefined'; + +function expectValid(date: Date | null, valid: boolean) { + if (date != null) { + expect(!isNaN(date.getTime())).toBe(valid, + `expected date to be ${valid ? 'valid' : 'invalid'}, was ${valid ? 'invalid' : 'valid'}`); + } else { + fail(`expected ${valid ? 'valid' : 'invalid'} date, was null`); + } +} + + describe('NativeDateAdapter', () => { const platform = new Platform(); let adapter: NativeDateAdapter; @@ -333,14 +344,15 @@ describe('NativeDateAdapter', () => { }); it('should create dates from valid ISO strings', () => { - expect(adapter.fromIso8601('1985-04-12T23:20:50.52Z')).not.toBeNull(); - expect(adapter.fromIso8601('1996-12-19T16:39:57-08:00')).not.toBeNull(); - expect(adapter.fromIso8601('1937-01-01T12:00:27.87+00:20')).not.toBeNull(); - expect(adapter.fromIso8601('2017-01-01')).not.toBeNull(); - expect(adapter.fromIso8601('2017-01-01T00:00:00')).not.toBeNull(); - expect(adapter.fromIso8601('1990-13-31T23:59:00Z')).toBeNull(); - expect(adapter.fromIso8601('1/1/2017')).toBeNull(); - expect(adapter.fromIso8601('2017-01-01T')).toBeNull(); + expectValid(adapter.coerceToDate('1985-04-12T23:20:50.52Z'), true); + expectValid(adapter.coerceToDate('1996-12-19T16:39:57-08:00'), true); + expectValid(adapter.coerceToDate('1937-01-01T12:00:27.87+00:20'), true); + expectValid(adapter.coerceToDate('2017-01-01'), true); + expectValid(adapter.coerceToDate('2017-01-01T00:00:00'), true); + expectValid(adapter.coerceToDate('1990-13-31T23:59:00Z'), false); + expectValid(adapter.coerceToDate('1/1/2017'), false); + expectValid(adapter.coerceToDate('2017-01-01T'), false); + expect(adapter.coerceToDate('')).toBeNull(); }); }); @@ -390,5 +402,4 @@ describe('NativeDateAdapter with LOCALE_ID override', () => { expect(adapter.getDayOfWeekNames('long')).toEqual(expectedValue); }); - }); diff --git a/src/lib/core/datetime/native-date-adapter.ts b/src/lib/core/datetime/native-date-adapter.ts index 8c672c477f61..e4e8dd62fa4b 100644 --- a/src/lib/core/datetime/native-date-adapter.ts +++ b/src/lib/core/datetime/native-date-adapter.ts @@ -7,8 +7,9 @@ */ import {Inject, Injectable, Optional} from '@angular/core'; -import {DateAdapter, MAT_DATE_LOCALE} from './date-adapter'; import {extendObject} from '../util/object-extend'; +import {DateAdapter, MAT_DATE_LOCALE} from './date-adapter'; + // TODO(mmalerba): Remove when we no longer support safari 9. /** Whether the browser supports the Intl API. */ @@ -219,16 +220,25 @@ export class NativeDateAdapter extends DateAdapter { ].join('-'); } - fromIso8601(iso8601String: string): Date | null { - // The `Date` constructor accepts formats other than ISO 8601, so we need to make sure the - // string is the right format first. - if (ISO_8601_REGEX.test(iso8601String)) { - let d = new Date(iso8601String); - if (this.isValid(d)) { - return d; + /** + * Coerces valid ISO 8601 strings (https://www.ietf.org/rfc/rfc3339.txt) to valid dates, empty + * string to null, all other values to an invalid date. + */ + coerceToDate(value: any): Date | null { + if (value == null || this.isDateInstance(value)) { + return value; + } + if (typeof value === 'string') { + if (!value) { + return null; + } + // The `Date` constructor accepts formats other than ISO 8601, so we need to make sure the + // string is the right format first. + if (ISO_8601_REGEX.test(value)) { + return new Date(value); } } - return null; + return new Date(NaN); } isDateInstance(obj: any) { diff --git a/src/lib/datepicker/coerce-date-property.spec.ts b/src/lib/datepicker/coerce-date-property.spec.ts index 52c15b36655e..f1d67d140771 100644 --- a/src/lib/datepicker/coerce-date-property.spec.ts +++ b/src/lib/datepicker/coerce-date-property.spec.ts @@ -23,11 +23,6 @@ describe('coerceDateProperty', () => { expect(coerceDateProperty(adapter, d)).toBe(d); }); - it('should pass through invalid date', () => { - const d = new Date(NaN); - expect(coerceDateProperty(adapter, d)).toBe(d); - }); - it('should pass through null and undefined', () => { expect(coerceDateProperty(adapter, null)).toBeNull(); expect(coerceDateProperty(adapter, undefined)).toBeUndefined(); @@ -51,4 +46,8 @@ describe('coerceDateProperty', () => { expect(() => coerceDateProperty(adapter, '1/1/2017')).toThrow(); expect(() => coerceDateProperty(adapter, 'hello')).toThrow(); }); + + it('should throw when given an invalid date', () => { + expect(() => coerceDateProperty(adapter, new Date(NaN))).toThrow(); + }); }); diff --git a/src/lib/datepicker/coerce-date-property.ts b/src/lib/datepicker/coerce-date-property.ts index 08fb8ffbb9a6..0db3123492dd 100644 --- a/src/lib/datepicker/coerce-date-property.ts +++ b/src/lib/datepicker/coerce-date-property.ts @@ -20,16 +20,10 @@ import {DateAdapter} from '@angular/material/core'; * @throws Throws when the value cannot be coerced. */ export function coerceDateProperty(adapter: DateAdapter, value: any): D | null { - if (typeof value === 'string') { - if (value == '') { - value = null; - } else { - value = adapter.fromIso8601(value) || value; - } + const d = adapter.coerceToDate(value); + if (adapter.isDateInstance(d) && !adapter.isValid(d as D)) { + throw Error(`Datepicker: Value must be either a date object recognized by the DateAdapter or ` + + `an ISO 8601 string. Instead got: ${value}`); } - if (value == null || adapter.isDateInstance(value)) { - return value; - } - throw Error(`Datepicker: Value must be either a date object recognized by the DateAdapter or ` + - `an ISO 8601 string. Instead got: ${value}`); + return d; } diff --git a/src/material-moment-adapter/adapter/moment-date-adapter.spec.ts b/src/material-moment-adapter/adapter/moment-date-adapter.spec.ts index 032b5c4c47bf..6b61ab53f6c9 100644 --- a/src/material-moment-adapter/adapter/moment-date-adapter.spec.ts +++ b/src/material-moment-adapter/adapter/moment-date-adapter.spec.ts @@ -6,18 +6,29 @@ * found in the LICENSE file at https://angular.io/license */ -import {MomentDateAdapter} from './moment-date-adapter'; +import {LOCALE_ID} from '@angular/core'; import {async, inject, TestBed} from '@angular/core/testing'; -import {MomentDateModule} from './index'; import {DateAdapter, MAT_DATE_LOCALE} from '@angular/material'; -import {LOCALE_ID} from '@angular/core'; import * as moment from 'moment'; +import {Moment} from 'moment'; +import {MomentDateModule} from './index'; +import {MomentDateAdapter} from './moment-date-adapter'; // Month constants for more readable tests. const JAN = 0, FEB = 1, MAR = 2, DEC = 11; +function expectValid(date: Moment | null, valid: boolean) { + if (date != null) { + expect(date.isValid()).toBe(valid, + `expected date to be ${valid ? 'valid' : 'invalid'}, was ${valid ? 'invalid' : 'valid'}`); + } else { + fail(`expected ${valid ? 'valid' : 'invalid'} date, was null`); + } +} + + describe('MomentDateAdapter', () => { let adapter: MomentDateAdapter; @@ -309,12 +320,13 @@ describe('MomentDateAdapter', () => { expect(adapter.isDateInstance(d)).toBe(false); }); - it('should create dates from valid ISO strings', () => { - expect(adapter.fromIso8601('1985-04-12T23:20:50.52Z')).not.toBeNull(); - expect(adapter.fromIso8601('1996-12-19T16:39:57-08:00')).not.toBeNull(); - expect(adapter.fromIso8601('1937-01-01T12:00:27.87+00:20')).not.toBeNull(); - expect(adapter.fromIso8601('1990-13-31T23:59:00Z')).toBeNull(); - expect(adapter.fromIso8601('1/1/2017')).toBeNull(); + it('should create valid dates from valid ISO strings', () => { + expectValid(adapter.coerceToDate('1985-04-12T23:20:50.52Z'), true); + expectValid(adapter.coerceToDate('1996-12-19T16:39:57-08:00'), true); + expectValid(adapter.coerceToDate('1937-01-01T12:00:27.87+00:20'), true); + expectValid(adapter.coerceToDate('1990-13-31T23:59:00Z'), false); + expectValid(adapter.coerceToDate('1/1/2017'), false); + expect(adapter.coerceToDate('')).toBeNull(); }); it('setLocale should not modify global moment locale', () => { diff --git a/src/material-moment-adapter/adapter/moment-date-adapter.ts b/src/material-moment-adapter/adapter/moment-date-adapter.ts index cbf42b5b2199..8925f0e3226e 100644 --- a/src/material-moment-adapter/adapter/moment-date-adapter.ts +++ b/src/material-moment-adapter/adapter/moment-date-adapter.ts @@ -8,14 +8,14 @@ import {Inject, Injectable, Optional} from '@angular/core'; import {DateAdapter, MAT_DATE_LOCALE} from '@angular/material'; - // Depending on whether rollup is used, moment needs to be imported differently. // Since Moment.js doesn't have a default export, we normally need to import using the `* as` // syntax. However, rollup creates a synthetic default module and we thus need to import it using // the `default as` syntax. // TODO(mmalerba): See if we can clean this up at some point. -import {default as _rollupMoment, Moment} from 'moment'; import * as _moment from 'moment'; +import {default as _rollupMoment, Moment} from 'moment'; + const moment = _rollupMoment || _moment; @@ -174,9 +174,24 @@ export class MomentDateAdapter extends DateAdapter { return this.clone(date).format(); } - fromIso8601(iso8601String: string): Moment | null { - let d = moment(iso8601String, moment.ISO_8601).locale(this.locale); - return this.isValid(d) ? d : null; + /** + * Coerces valid ISO 8601 strings (https://www.ietf.org/rfc/rfc3339.txt) and valid Date objects to + * valid dates, empty string to null, all other values to an invalid date. + */ + coerceToDate(value: any): Moment | null { + if (value == null || this.isDateInstance(value)) { + return value; + } + if (value instanceof Date) { + return moment(value); + } + if (typeof value === 'string') { + if (!value) { + return null; + } + return moment(value, moment.ISO_8601).locale(this.locale); + } + return moment.invalid(); } isDateInstance(obj: any): boolean {