rusackas opened a new pull request, #43909:
URL: https://github.com/apache/superset/pull/43909
### SUMMARY
Fast follow to #43839, rolling in the review points that were left open when
it merged.
**Shared `DateWithFormatter`.** The NaN guard #43839 added lived in two
byte-identical copies of the class, one in each table plugin, while the
`${value}` fallback in core's `stringifyTimeInput` only terminates because of
that guard (amaannawab923's and Bito's point). The class now lives in
`@superset-ui/core` next to the code that depends on it, both plugins import
the shared export, and the class tests moved to core. Each plugin keeps the
`formatColumnValue` case that exercises its own cell path. The termination
invariant is now pinned by a core test that feeds an unparseable
`DateWithFormatter` through `stringifyTimeInput`, and documented on both sides.
**Four-digit years.** CodeAnt flagged on #43839 that a digit-only string
like `"2017"` is read as epoch milliseconds. That was pre-existing (it came
from #38949, which restored pivot-table handling of stringified epoch
timestamps) so it was rightly kept out of that PR, but it is a real regression:
before #38949, `"2017"` formatted as January 2017, and after it formats as two
seconds past 1970. A bare four-digit string is the ISO 8601 year-only form,
which the spec requires every engine to parse as January 1st of that year, so
it now goes through `Date` parsing. Every other integer string keeps the epoch
handling. Verified in V8 that only the four-digit form is spec-guaranteed; five
and six digit strings parse as expanded years and eight-plus are invalid, so
the carve-out is deliberately narrow.
| input | `master` | this PR |
| --- | --- | --- |
| `"2017"` | `1970-01-01T00:00:02.017Z` | `2017-01-01T00:00:00.000Z` |
| `" 1987 "` | `1970-01-01T00:00:01.987Z` | `1987-01-01T00:00:00.000Z` |
| `"1704067200000"` | `2024-01-01T00:00:00.000Z` |
`2024-01-01T00:00:00.000Z` |
| `"00:01:54"` | `00:01:54` | `00:01:54` |
No plugin package public API changes: `DateWithFormatter` was never exported
from either plugin's index.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A
### TESTING INSTRUCTIONS
```
cd superset-frontend
npx jest --runInBand packages/superset-ui-core/test/time-format
plugins/plugin-chart-table plugins/plugin-chart-ag-grid-table
```
For the year change by hand: put a string column of four-digit years in a
Table chart with a `%Y` time format set in the column config. On `master` every
cell renders `1970`; here they render the year.
### ADDITIONAL INFORMATION
- [x] Has associated issue: follow-up to #43839 (#34328), regression from
#38949
- [ ] Required feature flags:
- [x] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_015XdpWLXhUs7r8d4F2R5D6F
--
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]
