Skip to content

Commit 74cceba

Browse files
shickskirjs
authored andcommittedFeb 24, 2025·
feat(common): throw error for suspicious date patterns (#59798)
Adjusts the date pipe and formatDate function to detect suspicious usages of the week-numbering year formatter without including the week number, as this is often confused for the calendar year and likely to result in incorrect results near New Years, meaning that those issues aren't typically caught during development. This commit starts throwing a development-only error to reveal this issue right away. BREAKING CHANGE: Using the `Y` formatter (week-numbering year) without also including `w` (week number) is now detected as suspicious date pattern, as `y` is typically intended. PR Close #59798
1 parent e4ef74d commit 74cceba

File tree

4 files changed

+65
-13
lines changed

4 files changed

+65
-13
lines changed
 

‎goldens/public-api/common/errors.api.md

+2
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ export const enum RuntimeErrorCode {
3737
// (undocumented)
3838
REQUIRED_INPUT_MISSING = 2954,
3939
// (undocumented)
40+
SUSPICIOUS_DATE_FORMAT = 2300,
41+
// (undocumented)
4042
TOO_MANY_PRELOADED_IMAGES = 2961,
4143
// (undocumented)
4244
TOO_MANY_PRIORITY_ATTRIBUTES = 2966,

‎packages/common/src/errors.ts

+3
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ export const enum RuntimeErrorCode {
2121
// NgForOf errors
2222
NG_FOR_MISSING_DIFFER = -2200,
2323

24+
// I18n errors
25+
SUSPICIOUS_DATE_FORMAT = 2300,
26+
2427
// Keep 2800 - 2900 for Http Errors.
2528

2629
// Image directive errors

‎packages/common/src/i18n/format_date.ts

+33
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9+
import {
10+
ɵRuntimeError as RuntimeError,
11+
ɵformatRuntimeError as formatRuntimeError,
12+
} from '@angular/core';
13+
914
import {
1015
FormatWidth,
1116
FormStyle,
@@ -24,6 +29,7 @@ import {
2429
Time,
2530
TranslationWidth,
2631
} from './locale_data_api';
32+
import {RuntimeErrorCode} from '../errors';
2733

2834
export const ISO8601_DATE_REGEX =
2935
/^(\d{4,})-?(\d\d)-?(\d\d)(?:T(\d\d)(?::?(\d\d)(?::?(\d\d)(?:\.(\d+))?)?)?(Z|([+-])(\d\d):?(\d\d))?)?$/;
@@ -104,6 +110,10 @@ export function formatDate(
104110
}
105111
}
106112

113+
if (typeof ngDevMode === 'undefined' || ngDevMode) {
114+
assertValidDateFormat(parts);
115+
}
116+
107117
let dateTimezoneOffset = date.getTimezoneOffset();
108118
if (timezone) {
109119
dateTimezoneOffset = timezoneToOffset(timezone, dateTimezoneOffset);
@@ -123,6 +133,29 @@ export function formatDate(
123133
return text;
124134
}
125135

136+
/**
137+
* Asserts that the given date format is free from common mistakes. Throws an
138+
* error if one is found (except for the case of all "Y", in which case we just
139+
* log a warning). This should only be called in development mode.
140+
*/
141+
function assertValidDateFormat(parts: string[]) {
142+
if (parts.some((part) => /^Y+$/.test(part)) && !parts.some((part) => /^w+$/.test(part))) {
143+
// "Y" indicates "week-based year", which differs from the actual calendar
144+
// year for a few days around Jan 1 most years. Unless "w" is also
145+
// present (e.g. a date like "2024-W52") this is likely a mistake. Users
146+
// probably meant "y" instead.
147+
const message = `Suspicious use of week-based year "Y" in date pattern "${parts.join(
148+
'',
149+
)}". Did you mean to use calendar year "y" instead?`;
150+
if (parts.length === 1) {
151+
// NOTE: allow "YYYY" with just a warning, since it's used in tests.
152+
console.error(formatRuntimeError(RuntimeErrorCode.SUSPICIOUS_DATE_FORMAT, message));
153+
} else {
154+
throw new RuntimeError(RuntimeErrorCode.SUSPICIOUS_DATE_FORMAT, message);
155+
}
156+
}
157+
}
158+
126159
/**
127160
* Create a new Date object with the given date value, and the time set to midnight.
128161
*

‎packages/common/test/i18n/format_date_spec.ts

+27-13
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,14 @@ describe('Format date', () => {
246246
BBBBB: 'at night',
247247
};
248248

249+
// Suppress console warnings for 'YYYY' patterns.
250+
const consoleError = console.error;
251+
spyOn(console, 'error').and.callFake((...args: unknown[]) => {
252+
if (!/Suspicious use of week-based year/.test(String(args))) {
253+
consoleError(...args);
254+
}
255+
});
256+
249257
Object.keys(dateFixtures).forEach((pattern: string) => {
250258
expectDateFormatAs(date, pattern, dateFixtures[pattern]);
251259
});
@@ -450,17 +458,23 @@ describe('Format date', () => {
450458

451459
// https://github.com/angular/angular/issues/38739
452460
it('should return correct ISO 8601 week-numbering year for dates close to year end/beginning', () => {
453-
expect(formatDate('2013-12-27', 'YYYY', 'en')).toEqual('2013');
454-
expect(formatDate('2013-12-29', 'YYYY', 'en')).toEqual('2013');
455-
expect(formatDate('2013-12-31', 'YYYY', 'en')).toEqual('2014');
461+
expect(formatDate('2013-12-27', `YYYY 'W'ww`, 'en')).toEqual('2013 W52');
462+
expect(formatDate('2013-12-29', `YYYY 'W'ww`, 'en')).toEqual('2013 W52');
463+
expect(formatDate('2013-12-31', `YYYY 'W'ww`, 'en')).toEqual('2014 W01');
456464

457465
// Dec. 31st is a Sunday, last day of the last week of 2023
458-
expect(formatDate('2023-12-31', 'YYYY', 'en')).toEqual('2023');
466+
expect(formatDate('2023-12-31', `YYYY 'W'ww`, 'en')).toEqual('2023 W52');
459467

460-
expect(formatDate('2010-01-02', 'YYYY', 'en')).toEqual('2009');
461-
expect(formatDate('2010-01-04', 'YYYY', 'en')).toEqual('2010');
462-
expect(formatDate('0049-01-01', 'YYYY', 'en')).toEqual('0048');
463-
expect(formatDate('0049-01-04', 'YYYY', 'en')).toEqual('0049');
468+
expect(formatDate('2010-01-02', `YYYY 'W'ww`, 'en')).toEqual('2009 W53');
469+
expect(formatDate('2010-01-04', `YYYY 'W'ww`, 'en')).toEqual('2010 W01');
470+
expect(formatDate('0049-01-01', `YYYY 'W'ww`, 'en')).toEqual('0048 W53');
471+
expect(formatDate('0049-01-04', `YYYY 'W'ww`, 'en')).toEqual('0049 W01');
472+
});
473+
474+
it('should throw an error when using YYYY incorrectly', () => {
475+
expect(() => formatDate('2013-12-31', `YYYY/MM/dd`, ɵDEFAULT_LOCALE_ID)).toThrowError(
476+
/.*Suspicious use of week-based year "Y".*/,
477+
);
464478
});
465479

466480
// https://github.com/angular/angular/issues/53813
@@ -480,11 +494,11 @@ describe('Format date', () => {
480494

481495
// https://github.com/angular/angular/issues/40377
482496
it('should format date with year between 0 and 99 correctly', () => {
483-
expect(formatDate('0098-01-11', 'YYYY', ɵDEFAULT_LOCALE_ID)).toEqual('0098');
484-
expect(formatDate('0099-01-11', 'YYYY', ɵDEFAULT_LOCALE_ID)).toEqual('0099');
485-
expect(formatDate('0100-01-11', 'YYYY', ɵDEFAULT_LOCALE_ID)).toEqual('0100');
486-
expect(formatDate('0001-01-11', 'YYYY', ɵDEFAULT_LOCALE_ID)).toEqual('0001');
487-
expect(formatDate('0000-01-11', 'YYYY', ɵDEFAULT_LOCALE_ID)).toEqual('0000');
497+
expect(formatDate('0098-01-11', `YYYY 'W'ww`, ɵDEFAULT_LOCALE_ID)).toEqual('0098 W02');
498+
expect(formatDate('0099-01-11', `YYYY 'W'ww`, ɵDEFAULT_LOCALE_ID)).toEqual('0099 W02');
499+
expect(formatDate('0100-01-11', `YYYY 'W'ww`, ɵDEFAULT_LOCALE_ID)).toEqual('0100 W02');
500+
expect(formatDate('0001-01-11', `YYYY 'W'ww`, ɵDEFAULT_LOCALE_ID)).toEqual('0001 W02');
501+
expect(formatDate('0000-01-11', `YYYY 'W'ww`, ɵDEFAULT_LOCALE_ID)).toEqual('0000 W02');
488502
});
489503

490504
// https://github.com/angular/angular/issues/26922

0 commit comments

Comments
 (0)
Please sign in to comment.