refactor: replace date-fns with safe date formatting functions across…#1053
refactor: replace date-fns with safe date formatting functions across…#1053
Conversation
… deployment components
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 22 minutes and 14 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
… improved reliability
There was a problem hiding this comment.
Pull request overview
Refactors deployment-related web UI components to use centralized “safe” date formatting utilities, preventing crashes when timestamps are null/invalid.
Changes:
- Added
apps/web/app/lib/date.tswith safe wrappers arounddate-fnsformatting/comparison helpers. - Replaced direct
date-fnsusage withsafeFormatDistance*helpers in several deployment components, adding UI fallbacks for null results. - Removed a duplicated local safe date helper in
EnvironmentProgressionDetailin favor of the shared utilities.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/web/app/routes/ws/deployments/page.$deploymentId.release-targets.$releaseTargetKey.tsx | Uses safeFormatDistanceToNow with fallbacks for evaluation timestamps. |
| apps/web/app/routes/ws/deployments/_components/release-targets/prometheus/Prometheus.tsx | Uses safe strict “time ago” formatting and renders placeholder on null. |
| apps/web/app/routes/ws/deployments/_components/release-targets/datadog/Datadog.tsx | Uses safe strict “time ago” formatting and renders placeholder on null. |
| apps/web/app/routes/ws/deployments/_components/release-targets/argocd/ArgoCD.tsx | Uses safe strict formatting for “Last Synced” with "unknown" fallback. |
| apps/web/app/routes/ws/deployments/_components/release-targets/VerificationMetricStatus.tsx | Switches to safe strict formatter for latest measurement timestamp. |
| apps/web/app/routes/ws/deployments/_components/environmentversiondecisions/rule-results/EnvironmentProgressionDetail.tsx | Removes inline safe helper and imports shared safe formatter. |
| apps/web/app/routes/ws/deployments/_components/environmentversiondecisions/rule-results/DeploymentWindowDetail.tsx | Replaces some date-fns calls with safe helpers, but leaves broken references (see comments). |
| apps/web/app/lib/date.ts | Introduces shared safe date helpers wrapping date-fns. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import { | ||
| safeFormatDistanceStrict, | ||
| safeFormatDistanceToNowStrict, | ||
| safeIsPast, | ||
| } from "~/lib/date"; |
There was a problem hiding this comment.
This file still references isPast and formatDistanceToNowStrict later in WindowRow (e.g. checks for ended/next window), but those date-fns imports were removed and the safe replacements are imported instead. This will cause a TypeScript compile error (unresolved identifiers) and also defeats the goal of using safe formatters. Update the remaining usages to safeIsPast / safeFormatDistanceToNowStrict (and handle null returns) or reintroduce the needed imports consistently.
| {evaluation.satisfiedAt != null && | ||
| safeFormatDistanceToNow(evaluation.satisfiedAt, { | ||
| addSuffix: true, | ||
| }) != null && ( | ||
| <span> | ||
| Satisfied{" "} | ||
| {safeFormatDistanceToNow(evaluation.satisfiedAt, { | ||
| addSuffix: true, | ||
| })} | ||
| </span> | ||
| )} |
There was a problem hiding this comment.
safeFormatDistanceToNow(evaluation.satisfiedAt, { addSuffix: true }) is computed twice (once in the conditional and again for rendering). Consider computing it once into a local variable and reusing it to avoid duplicate parsing/formatting and ensure the displayed value matches the conditional check.
| {evaluation.satisfiedAt != null && | |
| safeFormatDistanceToNow(evaluation.satisfiedAt, { | |
| addSuffix: true, | |
| }) != null && ( | |
| <span> | |
| Satisfied{" "} | |
| {safeFormatDistanceToNow(evaluation.satisfiedAt, { | |
| addSuffix: true, | |
| })} | |
| </span> | |
| )} | |
| {(() => { | |
| const satisfiedAtDistance = | |
| evaluation.satisfiedAt == null | |
| ? null | |
| : safeFormatDistanceToNow(evaluation.satisfiedAt, { | |
| addSuffix: true, | |
| }); | |
| return ( | |
| satisfiedAtDistance != null && ( | |
| <span> | |
| Satisfied {satisfiedAtDistance} | |
| </span> | |
| ) | |
| ); | |
| })()} |
…ource aggregates and workflow components
… deployment components