Skip to content

pref(grid): datetime formatter uses dayjs#2090

Open
ptma wants to merge 3 commits into
t8y2:mainfrom
ptma:pref_datetime_formatter
Open

pref(grid): datetime formatter uses dayjs#2090
ptma wants to merge 3 commits into
t8y2:mainfrom
ptma:pref_datetime_formatter

Conversation

@ptma

@ptma ptma commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

变更说明

优化时间戳列格式化使用 dayjs, 除预设的格式外,允许用户输入自定义的时间日期格式。

由于时间格式化测试用例与系统环境的时区相关, 在 pnpm 测试环境注入了环境变量 process.env.TZ = "Asia/Shanghai";

变更类型

  • 新功能
  • Bug 修复
  • 性能优化
  • 代码重构
  • 文档更新
  • CI / 构建

涉及前端

涉及列格式化中, Unix 时间戳类型的格式化选项及 DataGrid 输出。
由于新增了 dayjs , 需要执行

pnpm install
  • 本 PR 涉及前端改动,已附截图/录屏(见下方)
image

验证

  • pnpm check 通过
  • cargo check --no-default-features 通过
  • 相关测试通过

关联 Issue

@t8y2 t8y2 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR #2090: pref(grid): datetime formatter uses dayjs

+169/-80, 15 files | CI: frontend/checks passed, backend jobs skipped | Mergeable: MERGEABLE

🔴 必须修复

  • apps/desktop/src/lib/columnFormatter.ts:106-114 regresses compact numeric date strings. auto now treats every all-digit string as a Unix timestamp, so the previously covered "20260514" case no longer falls back to the original value. The old isTimestampString guard only accepted 10- or 13-digit timestamp strings; that protection should be restored or replaced, and the removed "20260514" assertion should come back.

  • The same block formats non-numeric strings with dayjs(value).format(...) without checking validity. Day.js documents isValid() for detecting unparseable inputs, so values like arbitrary text can become formatted invalid dates instead of falling back to displayCellValue(value). Please check parsed.isValid() before returning the formatted string.

🟡 改进建议

  • apps/desktop/src/components/grid/DataGrid.vue:7990 has a typo: grid..formatterDatetimePatternEmpty. It is probably low-impact here because custom values are allowed, but it should be grid.formatterDatetimePatternEmpty.

  • apps/desktop/src/lib/columnFormatter.ts:10 includes duplicate ISO-like patterns. Consider deduplicating the preset list before merge.

✅ 做得好的地方

  • Existing saved datetime formatter configs are migrated by normalizeColumnFormatter with a default pattern.
  • i18n entries were added across the supported locale files.
  • Frontend CI passed and the PR is mergeable.

结论

Request changes. The feature direction is fine, but the datetime string parsing regression is user-visible and should be fixed before merge.

References: https://day.js.org/docs/en/parse/is-valid, https://day.js.org/docs/en/plugin/custom-parse-format

@ptma ptma requested a review from t8y2 June 30, 2026 07:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants