Skip to content

chore: add ESLint rules for date formatting and .collect() conventions#379

Merged
Israeltheminer merged 3 commits into
mainfrom
chore/eslint-rules-and-convention-fixes
Feb 6, 2026
Merged

chore: add ESLint rules for date formatting and .collect() conventions#379
Israeltheminer merged 3 commits into
mainfrom
chore/eslint-rules-and-convention-fixes

Conversation

@Israeltheminer
Copy link
Copy Markdown
Collaborator

@Israeltheminer Israeltheminer commented Feb 6, 2026

Add no-restricted-syntax rules to catch native toLocaleString/toLocaleDateString/ toLocaleTimeString usage (directing to useDateFormat hook or formatDate utility) and .collect() in Convex files (directing to for-await or paginated queries).

Migrate existing violations to use shared date formatting utilities and add justified eslint-disable comments for bounded .collect() usages. Fix Badge variant types in audit log table.

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Standardized date and timestamp formatting across the application for improved consistency and readability in API keys, audit logs, and chat components.
  • Chores

    • Added configuration rules to enforce consistent date formatting practices throughout the codebase.

Add no-restricted-syntax rules to catch native toLocaleString/toLocaleDateString/
toLocaleTimeString usage (directing to useDateFormat hook or formatDate utility)
and .collect() in Convex files (directing to for-await or paginated queries).

Migrate existing violations to use shared date formatting utilities and add
justified eslint-disable comments for bounded .collect() usages. Fix Badge
variant types in audit log table.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Feb 6, 2026

📝 Walkthrough

Walkthrough

This pull request centralizes date formatting across the codebase by introducing a useDateFormat hook and ESLint rules to enforce its usage. Changes include: replacing local date formatting implementations in multiple component files with the shared hook, removing inline date formatting helpers, adding ESLint rules to restrict direct use of toLocaleString, toLocaleDateString, and toLocaleTimeString methods, and restricting the collect() function in Convex workflow files. Supporting comment additions clarify intent in specific code sections.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main changes: adding ESLint rules for date formatting conventions and .collect() restrictions, which is the core objective reflected across all modified files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/eslint-rules-and-convention-fixes

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
services/platform/app/features/settings/audit-logs/components/audit-log-table.tsx (1)

86-97: 🧹 Nitpick | 🔵 Trivial

Badge variant mapping looks correct; consider extracting for readability.

The nested ternary on line 90 is functional but dense. A small helper or map would improve scanability, especially if more statuses are added later.

♻️ Optional: extract status-to-variant mapping
+const statusVariantMap: Record<string, 'green' | 'yellow' | 'destructive'> = {
+  success: 'green',
+  denied: 'yellow',
+};
+
 cell: ({ row }) => {
   const status = row.original.status;
   return (
     <Badge
-      variant={status === 'success' ? 'green' : status === 'denied' ? 'yellow' : 'destructive'}
+      variant={statusVariantMap[status] ?? 'destructive'}
     >
       {status}
     </Badge>
   );
 },
services/platform/app/features/settings/api-keys/components/api-key-table.tsx (1)

52-80: 🧹 Nitpick | 🔵 Trivial

No date format preset specified — intentional?

All three formatDate calls here omit the preset argument, while audit-log-table.tsx (line 127) and human-input-request-card.tsx (line 243) pass 'long'. If the default preset produces a shorter/different format that's suitable for table cells, this is fine. Otherwise, consider aligning on an explicit preset for consistency across the app.

🤖 Fix all issues with AI agents
In `@services/platform/app/features/chat/components/human-input-request-card.tsx`:
- Line 243: Remove the unnecessary new Date() wrapper around timestamp in the
human input card and align the preset used by formatDate with the other
component: replace formatDate(new Date(timestamp), 'long') with either
formatDate(timestamp) to use the default ('medium') preset (matching
api-key-table's formatDate(row.original.createdAt)) or explicitly use
formatDate(timestamp, 'medium'); if you intentionally need the 'long' preset for
different visual context, keep 'long' but add a short comment in
human-input-request-card explaining why it differs from api-key-table to make
the divergence explicit.

In
`@services/platform/app/features/settings/api-keys/components/api-key-table.tsx`:
- Line 94: The useDateFormat hook returns new function references (formatDate,
formatDateSmartWithLocale, formatDateHeaderWithLocale, formatRelative) each
render which invalidates memoization where formatDate is a dependency; fix this
by wrapping each exported formatter in useCallback inside useDateFormat (e.g.,
create formatDateWithLocale, formatDateSmartWithLocale,
formatDateHeaderWithLocale, formatRelative using useCallback and listing their
real deps like locale and any t/locale-derived values) so their references are
stable and memoized consumers (such as the columns memo in api-key-table.tsx)
won't rerun on every render.

In `@services/platform/eslint.config.mjs`:
- Around line 65-77: The override for convex files replaces the earlier
no-restricted-syntax array, dropping the toLocale* selectors; update the convex
override so its rules.no-restricted-syntax array merges the existing
date-formatting selectors with the collect selector instead of replacing them.
Locate the rule key "no-restricted-syntax" in the convex-specific config object
(the block containing the selector
'CallExpression[callee.property.name="collect"]') and add the three toLocale
selectors (e.g., a selector matching
CallExpression[callee.property.name=/toLocale(Date|Time)?String/]) into that
same array so both restrictions apply to convex/**/*.ts files.
- Around line 57-61: The current ESLint rule using the AST selector
CallExpression[callee.property.name="toLocaleString"] falsely flags number
formatting too; update the rule so it only targets Date usages (e.g., restrict
the selector to calls on Date instances like
CallExpression[callee.object.type="Identifier"][callee.property.name="toLocaleString"]
with an additional type check for Date) or split into two rules (one for dates
that suggests useDateFormat()/formatDate() and one for numeric formatting with
an appropriate message), and if you keep a broad selector, at minimum change the
message to mention both date and number formatting to avoid misleading guidance.

Comment thread services/platform/eslint.config.mjs
Comment thread services/platform/eslint.config.mjs
…d-convention-fixes

# Conflicts:
#	services/platform/app/features/chat/components/human-input-request-card.tsx
#	services/platform/app/features/settings/audit-logs/components/audit-log-table.tsx
#	services/platform/lib/utils/onedrive-helpers.ts
- Remove duplicate useDateFormat import in api-key-table
- Update ESLint messages to reference useFormatDate() hook
- Merge date formatting selectors into Convex ESLint block to
  prevent flat config override from silently dropping them
@Israeltheminer Israeltheminer merged commit 44eafb3 into main Feb 6, 2026
2 checks passed
@Israeltheminer Israeltheminer deleted the chore/eslint-rules-and-convention-fixes branch February 6, 2026 16:12
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.

1 participant