Repository navigation
Conversation
Contributor
Code Metrics Report
Details | | main (94f6a5a) | #589 (67445cf) | +/- |
|---------------------|----------------|----------------|-------|
+ | Coverage | 88.1% | 88.4% | +0.2% |
| Files | 207 | 206 | -1 |
| Lines | 6176 | 6175 | -1 |
+ | Covered | 5446 | 5460 | +14 |
+ | Test Execution Time | 2m20s | 2m13s | -7s |Code coverage of files in pull request scope (87.4% → 88.4%, patch 98.5%)
Reported by octocov |
Contributor
Author
|
@copilot resolve the merge conflicts in this pull request |
…o fix/collection-datetime-timezone # Conflicts: # docs-manifest.json Co-authored-by: IzumiSy <982850+IzumiSy@users.noreply.github.com>
Contributor
Resolved the merge conflict and merged |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Follow-up to #546, stacked on
fix/datetime-filter-rfc3339so this PR contains only the remaining collection and cell timezone fixes.The datetime filter toolbar serializes edited values as RFC 3339 instants, but initial filters, bookmarked URLs, and saved views can still send legacy timezone-less values before anyone re-applies a filter. Built-in date cells and the cell menu also use browser-local interpretation in paths where the toolbar uses the configured AppShell timezone. When those zones differ, the displayed value and generated query can disagree.
Design Decision
Normalize at the collection state boundary
Use existing
tableMetadatato identify datetime fields and normalize initial state,addFilter, andsetFiltersbefore values reach queries or persistence callbacks. This covers URL and saved-view restoration without a second conversion in the URL parser or a mount effect that runs after the first query.Hooks without metadata remain unchanged: field names and string values are not enough to identify datetime fields safely. No new public configuration is introduced.
Preserve instants and reject invalid input
Interpret supported timezone-less ISO datetime strings in the AppShell timezone while preserving the instant represented by explicit offsets. Reuse the existing temporal conversion logic from a shared internal helper for scalar values, arrays, and range bounds. Validate string inputs before permissive display parsing; invalid metadata-backed datetime filters throw
TypeErrorinstead of silently dropping constraints, and rejected updates preserve existing state.Align built-in cells without shifting calendar dates
Pass the AppShell timezone into built-in timestamp rendering and cell-menu filter conversion. Format date-only values through a timezone-neutral branch so a calendar date stays unchanged, including dates skipped by a timezone transition. Custom renderers remain caller-owned.
Reuse column-scoped cell renderers
Constructing
Intl.DateTimeFormatis substantially more expensive than using an existing formatter. Creating it inside each cell's render repeats that setup for every row and rerender, even though the formatting settings are shared across the column.Date renderer instances retain lazily created
Intl.DateTimeFormatobjects for UTC calendar dates and AppShell-timezone timestamps. Memoize the built-in renderer instances per column so all rows share these formatters and row-only rerenders do not construct them again. Rebuild the renderer set when the ordered column array or AppShell timezone changes.Keep the cache local to each DataTable rather than introducing a global cache. Date parsing and timezone conversion still happen per value; custom renderers retain precedence and are not cached by this mechanism.
Local microbenchmark
A single local Node.js v24.21.0 run measured each operation over 5,000 calls, after 200 warm-up calls:
parseDate(...).toDate("UTC")America/New_YorkIntl.DateTimeFormatand format on every callIntl.DateTimeFormatinstanceThe two formatter cases used identical
en-USshort-month/numeric-day/numeric-year options andAmerica/New_York. These are directional, single-run microbenchmark results, not end-to-end DataTable, React render, or browser timings. They motivate avoiding repeated formatter construction; they do not imply the same speedup for the whole table, and parsing/timezone conversion costs remain.Summary