migrate: use layerstack format function #255
Conversation
WalkthroughThe pull request modifies date formatting across several frontend files. It removes the Changes
Possibly related PRs
Suggested reviewers
Poem
Warning Review ran into problems🔥 ProblemsGitHub Actions: Resource not accessible by integration - https://docs.github.com/rest/actions/workflow-runs#list-workflow-runs-for-a-repository. Please grant the required permissions to the CodeRabbit GitHub App under the organization or repository settings. ✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
frontend/src/lib/util/format.ts
Outdated
|
|
||
| if (includeTime) { | ||
| Object.assign(options, { | ||
| export function formatDate(value: string | number): string { |
There was a problem hiding this comment.
In my ECharts => LayerChart PR I also updated formatDate() to take in a Date | null | undefined along with string | number. Would you want to include that in this PR?
There was a problem hiding this comment.
Looks good. I brought in the null | undefined, added a check for undefined, and also brought in your code which checks if it is a date before converting.
I think this should cover our bases
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (4)
frontend/src/lib/components/JobOverviewChart.svelte(1 hunks)frontend/src/lib/util/format.ts(1 hunks)frontend/src/routes/joins/[slug]/job-tracking/+page.svelte(3 hunks)frontend/src/routes/joins/[slug]/observability/drift/+page.svelte(1 hunks)
🔇 Additional comments (4)
frontend/src/lib/components/JobOverviewChart.svelte (1)
8-11: LGTM! Props cleanup.Clean removal of formatDate from props.
frontend/src/routes/joins/[slug]/job-tracking/+page.svelte (2)
68-76: LGTM! Consistent date formatting.Clean implementation using layerstack format.
191-191: LGTM! Props update.Correctly removed formatDate prop.
frontend/src/routes/joins/[slug]/observability/drift/+page.svelte (1)
172-172: LGTM! Consistent date formatting.Using centralized formatDate utility.
| import { format, PeriodType } from '@layerstack/utils'; | ||
|
|
||
| /** | ||
| * Formats a timestamp into a human-readable date string | ||
| * Formats a timestamp into a human-readable date string. | ||
| * For example: Dec 31, 2022, 5:00:00 PM | ||
| */ | ||
| export function formatDate(value: string | number, includeTime: boolean = true): string { | ||
| const options: Intl.DateTimeFormatOptions = { | ||
| month: 'short', | ||
| day: 'numeric', | ||
| year: 'numeric' | ||
| }; | ||
|
|
||
| if (includeTime) { | ||
| Object.assign(options, { | ||
| export function formatDate(value: string | number): string { | ||
| return format(new Date(value), PeriodType.Custom, { | ||
| custom: { | ||
| month: 'short', | ||
| day: 'numeric', | ||
| year: 'numeric', | ||
| hour: 'numeric', | ||
| minute: '2-digit', | ||
| second: '2-digit', | ||
| hour12: true | ||
| }); | ||
| } | ||
|
|
||
| return new Date(value).toLocaleString('en-US', options); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
💡 Codebase verification
Using ^0.0.7 allows minor updates which can break in 0.x versions. Consider exact version 0.0.7.
🔗 Analysis chain
LGTM! Verify @layerstack/utils version.
The new implementation provides consistent date formatting.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check @layerstack/utils version
jq -r '.dependencies["@layerstack/utils"]' package.json
Length of output: 128
Script:
#!/bin/bash
# Find package.json files and check for @layerstack/utils
fd package.json --type f --exec sh -c 'echo "=== {} ==="; cat {} | grep -A1 "@layerstack/utils" || true'
Length of output: 202
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/src/lib/util/format.ts (1)
4-7: Add null behavior to docs.Add example of null/undefined return value.
/** * Formats a timestamp into a human-readable date string. * For example: Dec 31, 2022, 5:00:00 PM + * Returns empty string for null/undefined values. */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
frontend/src/lib/util/format.ts(1 hunks)
🔇 Additional comments (1)
frontend/src/lib/util/format.ts (1)
1-1: Lock @layerstack/utils version.Using
^0.0.7allows minor updates which can break in 0.x versions.
Summary
Switched over to using layerstack format function. This should preserve functionality (and treat dates in job tracker "as is" without converting for timezone.
Checklist
Summary by CodeRabbit
Changes in Date Formatting
formatDatefunction from multiple components.Impact on User Experience