Fix/issue 1673 datetime input parsing#1960
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request improves date and time string parsing in the DateTimeInput component across the Angular, Lit, and React renderers to align with the specification, adding corresponding unit tests. The reviewer identified a timezone shift bug in all three implementations when parsing date-only strings for a datetime-local input type, which causes the fallback parser to mix UTC dates with local timezones. To resolve this, the reviewer suggested refactoring the normalizeDateTimeValue helper function to avoid fallback parsing when a literal date is already extracted, and to make the date and time regular expressions more robust for single-digit values.
| function normalizeDateTimeValue(value: unknown, type: string): string { | ||
| if (value === null || value === undefined || value === '') return ''; | ||
|
|
||
| let strValue = ''; | ||
| if (value instanceof Date) { | ||
| strValue = value.toISOString(); | ||
| } else { | ||
| strValue = String(value).trim(); | ||
| } | ||
|
|
||
| if (!strValue) return ''; | ||
|
|
||
| let datePart = ''; | ||
| let timePart = ''; | ||
|
|
||
| // 1. Try literal extraction of YYYY-MM-DD or YYYY/MM/DD | ||
| const dateRegex = /(\d{4})[-/](\d{2})[-/](\d{2})/; | ||
| const dateMatch = strValue.match(dateRegex); | ||
| if (dateMatch) { | ||
| datePart = `${dateMatch[1]}-${dateMatch[2]}-${dateMatch[3]}`; | ||
| } | ||
|
|
||
| // 2. Try literal extraction of HH:mm | ||
| const timeRegex = /(\d{2}):(\d{2})(?::(\d{2}))?/; | ||
| const timeMatch = strValue.match(timeRegex); | ||
| if (timeMatch) { | ||
| timePart = `${timeMatch[1]}:${timeMatch[2]}`; | ||
| } | ||
|
|
||
| // 3. Fallback to Date object parsing if needed | ||
| const needDate = type === 'date' || type === 'datetime-local'; | ||
| const needTime = type === 'time' || type === 'datetime-local'; | ||
|
|
||
| if ((needDate && !datePart) || (needTime && !timePart)) { | ||
| let parsedDate: Date; | ||
| const num = Number(strValue); | ||
| if (strValue !== '' && !isNaN(num)) { | ||
| parsedDate = new Date(num); | ||
| } else { | ||
| parsedDate = new Date(strValue); | ||
| } | ||
|
|
||
| if (!isNaN(parsedDate.getTime())) { | ||
| if (!datePart) { | ||
| const y = parsedDate.getFullYear(); | ||
| const m = String(parsedDate.getMonth() + 1).padStart(2, '0'); | ||
| const d = String(parsedDate.getDate()).padStart(2, '0'); | ||
| datePart = `${y}-${m}-${d}`; | ||
| } | ||
| if (!timePart) { | ||
| const h = String(parsedDate.getHours()).padStart(2, '0'); | ||
| const min = String(parsedDate.getMinutes()).padStart(2, '0'); | ||
| timePart = `${h}:${min}`; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // If timePart is empty but we need it and datePart is present, default to '00:00' | ||
| if (type === 'datetime-local' && datePart && !timePart) { | ||
| timePart = '00:00'; | ||
| } | ||
|
|
||
| switch (type) { | ||
| case 'date': | ||
| return datePart; | ||
| case 'time': | ||
| return timePart; | ||
| case 'datetime-local': | ||
| return datePart && timePart ? `${datePart}T${timePart}` : ''; | ||
| } | ||
| return ''; | ||
| } |
There was a problem hiding this comment.
There is a timezone shift bug when parsing date-only strings (like "2026-03-20") when type is "datetime-local".
The Issue
- dateRegex successfully extracts datePart = '2026-03-20', but timePart remains empty.
- Since needTime is true, the fallback block is executed.
- new Date('2026-03-20') is parsed. According to the ECMAScript specification, date-only ISO strings are parsed as UTC midnight.
- When parsedDate.getHours() and parsedDate.getMinutes() are called, they return the hours and minutes in the local timezone of the environment (e.g., 19:00 in UTC-5).
- This mixes the UTC date with the local timezone-shifted time, producing an incorrect hybrid value like '2026-03-20T19:00' and completely bypassing the intended '00:00' default.
The Solution
Only perform the Date fallback parsing if we don't already have a literally extracted datePart (unless the input is a numeric timestamp, in which case we want to parse both). This ensures that date-only strings do not get timezone-shifted times.
Additionally, the current dateRegex and timeRegex require exactly two digits for months, days, and hours (e.g., \d{2}). This will fail to match single-digit values like "9:00" or "2026-3-2". We can make the regexes more robust by matching 1 or 2 digits (\d{1,2}) and padding them with leading zeros.
function normalizeDateTimeValue(value: unknown, type: string): string {
if (value === null || value === undefined || value === '') return '';
let strValue = '';
if (value instanceof Date) {
strValue = value.toISOString();
} else {
strValue = String(value).trim();
}
if (!strValue) return '';
let datePart = '';
let timePart = '';
// 1. Try literal extraction of YYYY-MM-DD or YYYY/MM/DD (supporting 1 or 2 digit month/day)
const dateRegex = /(\d{4})[-/](\d{1,2})[-/](\d{1,2})/;
const dateMatch = strValue.match(dateRegex);
if (dateMatch) {
datePart = `${dateMatch[1]}-${dateMatch[2].padStart(2, '0')}-${dateMatch[3].padStart(2, '0')}`;
}
// 2. Try literal extraction of HH:mm (supporting 1 or 2 digit hour)
const timeRegex = /(\d{1,2}):(\d{2})(?::(\d{2}))?/;
const timeMatch = strValue.match(timeRegex);
if (timeMatch) {
timePart = `${timeMatch[1].padStart(2, '0')}:${timeMatch[2]}`;
}
// 3. Fallback to Date object parsing if needed
const needDate = type === 'date' || type === 'datetime-local';
const needTime = type === 'time' || type === 'datetime-local';
if ((needDate && !datePart) || (needTime && !timePart)) {
const isTimestamp = strValue !== '' && !isNaN(Number(strValue));
// Only fallback to Date parsing if we don't have a literal datePart,
// or if the input is a numeric timestamp.
if (!datePart || isTimestamp) {
let parsedDate: Date;
if (isTimestamp) {
parsedDate = new Date(Number(strValue));
} else {
parsedDate = new Date(strValue);
}
if (!isNaN(parsedDate.getTime())) {
if (!datePart) {
const y = parsedDate.getFullYear();
const m = String(parsedDate.getMonth() + 1).padStart(2, '0');
const d = String(parsedDate.getDate()).padStart(2, '0');
datePart = `${y}-${m}-${d}`;
}
if (!timePart) {
const h = String(parsedDate.getHours()).padStart(2, '0');
const min = String(parsedDate.getMinutes()).padStart(2, '0');
timePart = `${h}:${min}`;
}
}
}
}
// If timePart is empty but we need it and datePart is present, default to '00:00'
if (type === 'datetime-local' && datePart && !timePart) {
timePart = '00:00';
}
switch (type) {
case 'date':
return datePart;
case 'time':
return timePart;
case 'datetime-local':
return datePart && timePart ? `${datePart}T${timePart}` : '';
}
return '';
}References
- Maintain consistent behavior and limitations across different framework renderers (such as React, Lit, and Angular) rather than implementing complex, custom parsing logic (like robust date/time parsing) in a single renderer.
There was a problem hiding this comment.
This is indeed a problem.
There was a problem hiding this comment.
Yeah, date parsing in javascript is a mess. @gspencergoog I wonder if we should just let users inject their date parsing/formatting and have the framework always use UTC internally. The actual datetime input controls may be tricky though.
There was a problem hiding this comment.
Yeah, I originally only specified UTC variants for this control mainly because of this issue. Once you add in local time, stuff gets complicated.
Injecting their own formatting is a good idea, but it adds overhead to using the catalog. Is there a default we could provide them that would be useful? Ideally one from a well maintained package that we don't have to maintain...
There was a problem hiding this comment.
Injecting their own formatting is a good idea, but it adds overhead to using the catalog. Is there a default we could provide them that would be useful? Ideally one from a well maintained package that we don't have to maintain...
Yes, we could have something like @a2ui/markdown-it (imagine @a2ui/luxon or similar). I guess what library we end up using depends on what we want the date parsing/formatting API to be.
| function normalizeDateTimeValue(value: unknown, type: string): string { | ||
| if (value === null || value === undefined || value === '') return ''; | ||
|
|
||
| const hasT = value.includes('T'); | ||
| const split = value.split('T'); | ||
| let strValue = ''; | ||
| if (value instanceof Date) { | ||
| strValue = value.toISOString(); | ||
| } else { | ||
| strValue = String(value).trim(); | ||
| } | ||
|
|
||
| if (!strValue) return ''; | ||
|
|
||
| let datePart = ''; | ||
| let timePart = ''; | ||
|
|
||
| // 1. Try literal extraction of YYYY-MM-DD or YYYY/MM/DD | ||
| const dateRegex = /(\d{4})[-/](\d{2})[-/](\d{2})/; | ||
| const dateMatch = strValue.match(dateRegex); | ||
| if (dateMatch) { | ||
| datePart = `${dateMatch[1]}-${dateMatch[2]}-${dateMatch[3]}`; | ||
| } | ||
|
|
||
| // 2. Try literal extraction of HH:mm | ||
| const timeRegex = /(\d{2}):(\d{2})(?::(\d{2}))?/; | ||
| const timeMatch = strValue.match(timeRegex); | ||
| if (timeMatch) { | ||
| timePart = `${timeMatch[1]}:${timeMatch[2]}`; | ||
| } | ||
|
|
||
| // If the incoming value is not in ISO format (not hasT), use the incoming value. | ||
| // It might be just a date: '2010-07-11' or a time: '13:37', so we use value as a | ||
| // the fallback to the split. | ||
| const datePart = (hasT ? split[0] : value)?.substring(0, 10) ?? ''; | ||
| const timePart = (hasT ? split[1] : value)?.substring(0, 5) ?? ''; | ||
| // 3. Fallback to Date object parsing if needed | ||
| const needDate = type === 'date' || type === 'datetime-local'; | ||
| const needTime = type === 'time' || type === 'datetime-local'; | ||
|
|
||
| if ((needDate && !datePart) || (needTime && !timePart)) { | ||
| let parsedDate: Date; | ||
| const num = Number(strValue); | ||
| if (strValue !== '' && !isNaN(num)) { | ||
| parsedDate = new Date(num); | ||
| } else { | ||
| parsedDate = new Date(strValue); | ||
| } | ||
|
|
||
| if (!isNaN(parsedDate.getTime())) { | ||
| if (!datePart) { | ||
| const y = parsedDate.getFullYear(); | ||
| const m = String(parsedDate.getMonth() + 1).padStart(2, '0'); | ||
| const d = String(parsedDate.getDate()).padStart(2, '0'); | ||
| datePart = `${y}-${m}-${d}`; | ||
| } | ||
| if (!timePart) { | ||
| const h = String(parsedDate.getHours()).padStart(2, '0'); | ||
| const min = String(parsedDate.getMinutes()).padStart(2, '0'); | ||
| timePart = `${h}:${min}`; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // If timePart is empty but we need it and datePart is present, default to '00:00' | ||
| if (type === 'datetime-local' && datePart && !timePart) { | ||
| timePart = '00:00'; | ||
| } | ||
|
|
||
| switch (type) { | ||
| case 'date': | ||
| return datePart; | ||
| case 'time': | ||
| return timePart; | ||
| case 'datetime-local': | ||
| return `${datePart}T${timePart}`; | ||
| return datePart && timePart ? `${datePart}T${timePart}` : ''; | ||
| } | ||
| return ''; | ||
| } |
There was a problem hiding this comment.
There is a timezone shift bug when parsing date-only strings (like "2026-03-20") when type is "datetime-local".
The Issue
- dateRegex successfully extracts datePart = '2026-03-20', but timePart remains empty.
- Since needTime is true, the fallback block is executed.
- new Date('2026-03-20') is parsed. According to the ECMAScript specification, date-only ISO strings are parsed as UTC midnight.
- When parsedDate.getHours() and parsedDate.getMinutes() are called, they return the hours and minutes in the local timezone of the environment (e.g., 19:00 in UTC-5).
- This mixes the UTC date with the local timezone-shifted time, producing an incorrect hybrid value like '2026-03-20T19:00' and completely bypassing the intended '00:00' default.
The Solution
Only perform the Date fallback parsing if we don't already have a literally extracted datePart (unless the input is a numeric timestamp, in which case we want to parse both). This ensures that date-only strings do not get timezone-shifted times.
Additionally, the current dateRegex and timeRegex require exactly two digits for months, days, and hours (e.g., \d{2}). This will fail to match single-digit values like "9:00" or "2026-3-2". We can make the regexes more robust by matching 1 or 2 digits (\d{1,2}) and padding them with leading zeros.
function normalizeDateTimeValue(value: unknown, type: string): string {
if (value === null || value === undefined || value === '') return '';
let strValue = '';
if (value instanceof Date) {
strValue = value.toISOString();
} else {
strValue = String(value).trim();
}
if (!strValue) return '';
let datePart = '';
let timePart = '';
// 1. Try literal extraction of YYYY-MM-DD or YYYY/MM/DD (supporting 1 or 2 digit month/day)
const dateRegex = /(\d{4})[-/](\d{1,2})[-/](\d{1,2})/;
const dateMatch = strValue.match(dateRegex);
if (dateMatch) {
datePart = `${dateMatch[1]}-${dateMatch[2].padStart(2, '0')}-${dateMatch[3].padStart(2, '0')}`;
}
// 2. Try literal extraction of HH:mm (supporting 1 or 2 digit hour)
const timeRegex = /(\d{1,2}):(\d{2})(?::(\d{2}))?/;
const timeMatch = strValue.match(timeRegex);
if (timeMatch) {
timePart = `${timeMatch[1].padStart(2, '0')}:${timeMatch[2]}`;
}
// 3. Fallback to Date object parsing if needed
const needDate = type === 'date' || type === 'datetime-local';
const needTime = type === 'time' || type === 'datetime-local';
if ((needDate && !datePart) || (needTime && !timePart)) {
const isTimestamp = strValue !== '' && !isNaN(Number(strValue));
// Only fallback to Date parsing if we don't have a literal datePart,
// or if the input is a numeric timestamp.
if (!datePart || isTimestamp) {
let parsedDate: Date;
if (isTimestamp) {
parsedDate = new Date(Number(strValue));
} else {
parsedDate = new Date(strValue);
}
if (!isNaN(parsedDate.getTime())) {
if (!datePart) {
const y = parsedDate.getFullYear();
const m = String(parsedDate.getMonth() + 1).padStart(2, '0');
const d = String(parsedDate.getDate()).padStart(2, '0');
datePart = `${y}-${m}-${d}`;
}
if (!timePart) {
const h = String(parsedDate.getHours()).padStart(2, '0');
const min = String(parsedDate.getMinutes()).padStart(2, '0');
timePart = `${h}:${min}`;
}
}
}
}
// If timePart is empty but we need it and datePart is present, default to '00:00'
if (type === 'datetime-local' && datePart && !timePart) {
timePart = '00:00';
}
switch (type) {
case 'date':
return datePart;
case 'time':
return timePart;
case 'datetime-local':
return datePart && timePart ? `${datePart}T${timePart}` : '';
}
return '';
}References
- Maintain consistent behavior and limitations across different framework renderers (such as React, Lit, and Angular) rather than implementing complex, custom parsing logic (like robust date/time parsing) in a single renderer.
There was a problem hiding this comment.
This is also a problem. Also, it appears that we are doing similar work in these libraries: is there any opportunity to share code (maybe add some utility functions in the web_core lib)?
| function normalizeDateTimeValue(value: unknown, type: string): string { | ||
| if (value === null || value === undefined || value === '') return ''; | ||
|
|
||
| const hasT = value.includes('T'); | ||
| const split = value.split('T'); | ||
| let strValue = ''; | ||
| if (value instanceof Date) { | ||
| strValue = value.toISOString(); | ||
| } else { | ||
| strValue = String(value).trim(); | ||
| } | ||
|
|
||
| if (!strValue) return ''; | ||
|
|
||
| let datePart = ''; | ||
| let timePart = ''; | ||
|
|
||
| // 1. Try literal extraction of YYYY-MM-DD or YYYY/MM/DD | ||
| const dateRegex = /(\d{4})[-/](\d{2})[-/](\d{2})/; | ||
| const dateMatch = strValue.match(dateRegex); | ||
| if (dateMatch) { | ||
| datePart = `${dateMatch[1]}-${dateMatch[2]}-${dateMatch[3]}`; | ||
| } | ||
|
|
||
| // 2. Try literal extraction of HH:mm | ||
| const timeRegex = /(\d{2}):(\d{2})(?::(\d{2}))?/; | ||
| const timeMatch = strValue.match(timeRegex); | ||
| if (timeMatch) { | ||
| timePart = `${timeMatch[1]}:${timeMatch[2]}`; | ||
| } | ||
|
|
||
| // 3. Fallback to Date object parsing if needed | ||
| const needDate = type === 'date' || type === 'datetime-local'; | ||
| const needTime = type === 'time' || type === 'datetime-local'; | ||
|
|
||
| if ((needDate && !datePart) || (needTime && !timePart)) { | ||
| let parsedDate: Date; | ||
| const num = Number(strValue); | ||
| if (strValue !== '' && !isNaN(num)) { | ||
| parsedDate = new Date(num); | ||
| } else { | ||
| parsedDate = new Date(strValue); | ||
| } | ||
|
|
||
| const datePart = (hasT ? split[0] : value)?.substring(0, 10) ?? ''; | ||
| const timePart = (hasT ? split[1] : value)?.substring(0, 5) ?? ''; | ||
| if (!isNaN(parsedDate.getTime())) { | ||
| if (!datePart) { | ||
| const y = parsedDate.getFullYear(); | ||
| const m = String(parsedDate.getMonth() + 1).padStart(2, '0'); | ||
| const d = String(parsedDate.getDate()).padStart(2, '0'); | ||
| datePart = `${y}-${m}-${d}`; | ||
| } | ||
| if (!timePart) { | ||
| const h = String(parsedDate.getHours()).padStart(2, '0'); | ||
| const min = String(parsedDate.getMinutes()).padStart(2, '0'); | ||
| timePart = `${h}:${min}`; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // If timePart is empty but we need it and datePart is present, default to '00:00' | ||
| if (type === 'datetime-local' && datePart && !timePart) { | ||
| timePart = '00:00'; | ||
| } | ||
|
|
||
| switch (type) { | ||
| case 'date': | ||
| return datePart; | ||
| case 'time': | ||
| return timePart; | ||
| case 'datetime-local': | ||
| return `${datePart}T${timePart}`; | ||
| return datePart && timePart ? `${datePart}T${timePart}` : ''; | ||
| } | ||
| return ''; | ||
| } |
There was a problem hiding this comment.
There is a timezone shift bug when parsing date-only strings (like "2026-03-20") when type is "datetime-local".
The Issue
- dateRegex successfully extracts datePart = '2026-03-20', but timePart remains empty.
- Since needTime is true, the fallback block is executed.
- new Date('2026-03-20') is parsed. According to the ECMAScript specification, date-only ISO strings are parsed as UTC midnight.
- When parsedDate.getHours() and parsedDate.getMinutes() are called, they return the hours and minutes in the local timezone of the environment (e.g., 19:00 in UTC-5).
- This mixes the UTC date with the local timezone-shifted time, producing an incorrect hybrid value like '2026-03-20T19:00' and completely bypassing the intended '00:00' default.
The Solution
Only perform the Date fallback parsing if we don't already have a literally extracted datePart (unless the input is a numeric timestamp, in which case we want to parse both). This ensures that date-only strings do not get timezone-shifted times.
Additionally, the current dateRegex and timeRegex require exactly two digits for months, days, and hours (e.g., \d{2}). This will fail to match single-digit values like "9:00" or "2026-3-2". We can make the regexes more robust by matching 1 or 2 digits (\d{1,2}) and padding them with leading zeros.
function normalizeDateTimeValue(value: unknown, type: string): string {
if (value === null || value === undefined || value === '') return '';
let strValue = '';
if (value instanceof Date) {
strValue = value.toISOString();
} else {
strValue = String(value).trim();
}
if (!strValue) return '';
let datePart = '';
let timePart = '';
// 1. Try literal extraction of YYYY-MM-DD or YYYY/MM/DD (supporting 1 or 2 digit month/day)
const dateRegex = /(\d{4})[-/](\d{1,2})[-/](\d{1,2})/;
const dateMatch = strValue.match(dateRegex);
if (dateMatch) {
datePart = `${dateMatch[1]}-${dateMatch[2].padStart(2, '0')}-${dateMatch[3].padStart(2, '0')}`;
}
// 2. Try literal extraction of HH:mm (supporting 1 or 2 digit hour)
const timeRegex = /(\d{1,2}):(\d{2})(?::(\d{2}))?/;
const timeMatch = strValue.match(timeRegex);
if (timeMatch) {
timePart = `${timeMatch[1].padStart(2, '0')}:${timeMatch[2]}`;
}
// 3. Fallback to Date object parsing if needed
const needDate = type === 'date' || type === 'datetime-local';
const needTime = type === 'time' || type === 'datetime-local';
if ((needDate && !datePart) || (needTime && !timePart)) {
const isTimestamp = strValue !== '' && !isNaN(Number(strValue));
// Only fallback to Date parsing if we don't have a literal datePart,
// or if the input is a numeric timestamp.
if (!datePart || isTimestamp) {
let parsedDate: Date;
if (isTimestamp) {
parsedDate = new Date(Number(strValue));
} else {
parsedDate = new Date(strValue);
}
if (!isNaN(parsedDate.getTime())) {
if (!datePart) {
const y = parsedDate.getFullYear();
const m = String(parsedDate.getMonth() + 1).padStart(2, '0');
const d = String(parsedDate.getDate()).padStart(2, '0');
datePart = `${y}-${m}-${d}`;
}
if (!timePart) {
const h = String(parsedDate.getHours()).padStart(2, '0');
const min = String(parsedDate.getMinutes()).padStart(2, '0');
timePart = `${h}:${min}`;
}
}
}
}
// If timePart is empty but we need it and datePart is present, default to '00:00'
if (type === 'datetime-local' && datePart && !timePart) {
timePart = '00:00';
}
switch (type) {
case 'date':
return datePart;
case 'time':
return timePart;
case 'datetime-local':
return datePart && timePart ? `${datePart}T${timePart}` : '';
}
return '';
}
References
- Maintain consistent behavior and limitations across different framework renderers (such as React, Lit, and Angular) rather than implementing complex, custom parsing logic (like robust date/time parsing) in a single renderer.
|
@Varun-S10 thanks for the PR! One question: Why is this needed:
If we are doing:
Is it because of the fallback to using the |
@ditman, Thanks for the review. No, it's not because of the fallback in The reason for the rollover is that in
|
3 participants
Description
This PR fixes date and time parsing in the
DateTimeInputcomponent across Angular, Lit, and React (v0_9) so it works reliably and follows the A2UI spec.Before this change,
DateTimeInputused a simple string split (value.split('T')[0]) to get the date and time. Because browser input boxes (date,time, anddatetime-local) are very strict about format, this caused the browser to reject slightly different date formats or timestamps and show an empty (cleared) input box on screen.What this PR changes:
normalizeDateTimeValue(value, type)helper function across Angular, Lit, and React.YYYY-MM-DDandHH:mm) to read exact date and time strings without shifting the date across timezones.Dateparsing for timestamps or irregular formats, formatting them with leading zeros as required by HTML input boxes.30_live-invitation-builder.jsonfrom19:00:00Zto10:00:00Zso automated tests don't roll over to the next day in timezones like IST (India).Fixes #1673
Pre-launch Checklist
One time:
For this PR:
If you need help, consider asking for advice on the discussion board.