raphaelsales commented on code in PR #43839:
URL: https://github.com/apache/superset/pull/43839#discussion_r3930256094
##########
superset-frontend/packages/superset-ui-core/src/time-format/utils/stringifyTimeInput.ts:
##########
@@ -25,11 +25,22 @@ export default function stringifyTimeInput(
return `${value}`;
}
+ let time: Date;
if (typeof value === 'string') {
const trimmed = value.trim();
const isIntegerString = /^-?\d+$/.test(trimmed);
- return fn(new Date(isIntegerString ? Number(trimmed) : value));
+ time = new Date(isIntegerString ? Number(trimmed) : value);
Review Comment:
This doesn't change how numeric strings are parsed. The expression is
byte-identical to `master`:
```diff
- return fn(new Date(isIntegerString ? Number(trimmed) : value));
+ time = new Date(isIntegerString ? Number(trimmed) : value);
```
The regex and the `Number(trimmed)` branch are untouched. The only
difference is that the `Date` is assigned before being passed to `fn`, so its
validity can be checked first.
`"2017"` resolves to `1970-01-01T00:00:02.017Z` on `master` and on this
branch alike. Running both implementations over the same inputs:
| input | master | this PR |
| --- | --- | --- |
| `"2017"` | `1970-01-01T00:00:02.017Z` | `1970-01-01T00:00:02.017Z` |
| `"1987"` | `1970-01-01T00:00:01.987Z` | `1970-01-01T00:00:01.987Z` |
| `"1487071353000"` | `2017-02-14T11:22:33.000Z` |
`2017-02-14T11:22:33.000Z` |
| `"2017-02-14T11:22:33Z"` | `2017-02-14T11:22:33.000Z` |
`2017-02-14T11:22:33.000Z` |
| `"00:01:54"` | `NaN:NaN:NaN` | `00:01:54` |
The last row is the only behavioural change in this PR, and it is the bug
being fixed.
The `"2017"` → epoch behaviour is real, but it predates this change and is
orthogonal to #34328. Narrowing the integer-string branch — by length or by
range — would change how epoch-in-a-string values parse across every chart,
which shouldn't ride along in a rendering fix. Happy to open a separate issue
for it if maintainers think it's worth pursuing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
