fix(admin-ui): remove moment.js and standardize date handling with da…#2615
Merged
fix(admin-ui): remove moment.js and standardize date handling with da…#2615
Conversation
mo-auto
added
area-documentation
Contributor
📝 WalkthroughWalkthroughRemoves the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
admin-ui/plugins/user-management/components/User2FADevicesModal.tsx (1)
124-132: Avoid epoch date whenaddedOnis missing.
Using(item.addedOn || 0) * 1000renders1970-01-01for missing values, which is misleading. Prefer a guard and show-/empty instead.🛠️ Suggested fix
- dateAdded: formatDate((item.addedOn || 0) * 1000, 'YYYY-MM-DD HH:mm:ss'), + dateAdded: item?.addedOn + ? formatDate(item.addedOn * 1000, 'YYYY-MM-DD HH:mm:ss') + : '-',admin-ui/plugins/admin/components/Assets/JansAssetListPage.tsx (1)
79-96: Mutating outer-scopeletvariables insideuseCallbackwon't persist across renders.
memoLimitandmemoPattern(lines 58-59) areletvariables in the component scope. Reassigning them insidehandleOptionsChange(lines 83-85) modifies copies captured at callback creation time, but these changes won't persist or affect subsequent renders. The refresh action (lines 150-152) will use stale values.Additionally,
keyCodeis deprecated; prefer usingevent.key === 'Enter'instead.🐛 Suggested fix using refs or state
- let memoLimit: number = limit - let memoPattern: string | undefined = pattern + const memoLimitRef = React.useRef<number>(limit) + const memoPatternRef = React.useRef<string | undefined>(pattern) + + // Keep refs in sync with state + useEffect(() => { + memoLimitRef.current = limit + memoPatternRef.current = pattern + }, [limit, pattern])Then in
handleOptionsChange:const handleOptionsChange = useCallback( (event: React.ChangeEvent<HTMLInputElement> | React.KeyboardEvent<HTMLInputElement>) => { const target = event.target as HTMLInputElement if (target.name === 'limit') { - memoLimit = Number(target.value) + memoLimitRef.current = Number(target.value) } else if (target.name === 'pattern') { - memoPattern = String(target.value) || undefined - if ('keyCode' in event && event.keyCode === 13) { + memoPatternRef.current = String(target.value) || undefined + if ('key' in event && event.key === 'Enter') { const newOptions = { limit: limit, - pattern: memoPattern, + pattern: memoPatternRef.current, } dispatch(fetchJansAssets(newOptions)) } } }, [limit, dispatch], )
🤖 Fix all issues with AI agents
In `@admin-ui/plugins/auth-server/components/Clients/ClientActiveTokens.js`:
- Around line 204-209: The date format string used when formatting
item.expirationDate and item.creationDate is incorrect ('YYYY/DD/MM HH:mm:ss'
uses day and month swapped); update the format passed to formatDate in the
ClientActiveTokens component (the two places assigning expirationDate and
creationDate) to the intended format (e.g., 'YYYY/MM/DD HH:mm:ss' or 'DD/MM/YYYY
HH:mm:ss') so dates render correctly; ensure both occurrences use the same
corrected format string.
In `@admin-ui/plugins/user-management/components/UserDetailViewPage.tsx`:
- Around line 96-98: The ternary that checks typeof valueToShow === 'boolean' is
dead because valueToShow is always a string earlier in UserDetailViewPage
(assigned on lines ~69-76); remove the unreachable boolean branch and simplify
the expression to just convert valueToShow to a string (e.g., use
String(valueToShow)) where it is currently rendered, or alternatively adjust the
earlier assignments so valueToShow can be boolean if that was intended—update
the expression in the render (the ternary using valueToShow) accordingly.
In `@admin-ui/plugins/user-management/utils/attributeTransformUtils.ts`:
- Around line 48-55: The birthdate branch using attributeName === BIRTHDATE_ATTR
should validate and format strictly: replace the loose isValidDate(normalized)
check with a strict parse using dayjs with customParseFormat in strict mode (use
the same utility from dayjsUtils.ts) to both validate the date logic (reject
overflow like 2024-02-30) and produce the parsed instance for formatting; update
the logic in the attributeTransformUtils.ts block that currently tests
normalized with birthdatePattern and calls isValidDate/formatDate so it uses the
strict dayjs parse result for validation and then format the parsed instance to
'YYYY-MM-DD' to avoid timezone drift.
admin-ui/plugins/user-management/components/UserDetailViewPage.tsx
Outdated
Show resolved
Hide resolved
…ex into admin-ui-issue-2577
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@admin-ui/app/utils/dayjsUtils.ts`:
- Around line 58-61: Add a short JSDoc atop the isValidDate function stating it
uses dayjs(date).isValid() which is lenient (it normalizes overflow dates like
"2024-02-30") and therefore not suitable for strict format validation; recommend
callers use parseDateStrict for exact format adherence and mention the accepted
input types (string | number | Date | Dayjs | null | undefined) and that
null/undefined return false.
- Around line 118-120: Remove the redundant cloneDate function: delete the
export const cloneDate definition (the function named cloneDate returning
dayjs(date).clone()) from admin-ui/app/utils/dayjsUtils.ts because Day.js
instances are immutable and cloning is unnecessary; also remove any unused Dayjs
import or related unused references in that module to keep imports clean and
ensure no linter warnings about an unused symbol.
In `@admin-ui/plugins/user-management/components/UserDetailViewPage.tsx`:
- Around line 66-72: The current viewer branch treats dates leniently by calling
isValidDate(raw) and formatting with formatDate, causing overflow dates (e.g.,
"2024-02-30") to be normalized; replace that check with the strict parser
parseDateStrict(raw) so only strictly parsed dates are shown: call
parseDateStrict on raw (from data?.values?.[0]) and if it returns a valid date,
format it (via formatDate) into "YYYY-MM-DD" and assign to valueToShow,
otherwise set valueToShow = '' to match attributeTransformUtils behavior.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@admin-ui/app/utils/dayjsUtils.ts`:
- Around line 73-84: The createDate function currently performs lenient parsing
when a format is provided (it calls dayjs(date, format) without strict mode),
which differs from parseDateStrict; add a JSDoc comment above createDate that
clearly states: the accepted input types, that providing a format enables
lenient (non-strict) parsing, and that callers who need strict validation should
use parseDateStrict; mention the default behavior when date is null and that the
function returns a Dayjs instance.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@admin-ui/app/utils/dayjsUtils.ts`:
- Around line 97-104: The default format string in formatDate duplicates the
constant DATE_FORMATS.DATE_ONLY; update formatDate to use DATE_FORMATS.DATE_ONLY
as the default (adjust the function signature default and ensure DATE_FORMATS is
imported where formatDate is defined) so callers and tests use the centralized
constant for the date-only format.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@admin-ui/plugins/auth-server/components/Scopes/ScopeForm.tsx`:
- Around line 679-683: The defaultValue for the date field in ScopeForm.tsx uses
a redundant ternary checking scope.creationDate; replace the ternary with a
direct call to formatDate(scope.creationDate, 'YYYY-MM-DD HH:mm:ss') (remove the
?: branch) because the formatDate utility already handles null/undefined; update
the defaultValue in the JSX where scope.creationDate is used to rely solely on
formatDate.
- Around line 608-612: The defaultValue ternary can be simplified: remove the
explicit null/undefined check around scope.creationDate and pass
scope.creationDate directly into formatDate (which already returns '' for
null/undefined); update the defaultValue expression in ScopeForm (the block
using formatDate) to use formatDate(scope.creationDate, 'YYYY-MM-DD HH:mm:ss')
only, leaving the rest of the component (ScopeForm) unchanged.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@admin-ui/app/routes/Dashboards/DashboardPage.tsx`:
- Around line 35-41: The import alias formatDateDayjs is unclear; rename it to a
more descriptive identifier such as formatDayjsDate (or formatDateWithDayjs) in
the import from '@/utils/dayjsUtils' and update all usages in DashboardPage.tsx
to that new name (references include formatDateDayjs and related dayjs helpers
like createDate, isAfterDate, isBeforeDate). Ensure the other formatDate import
from Utils/Util remains unchanged to avoid collisions, or migrate callers to the
dayjs helper if you choose to consolidate.
- Around line 342-348: Add a centralized month format constant to DATE_FORMATS
in dayjsUtils.ts (e.g., MONTH_KEY or MONTH) and replace the inline string
'YYYYMM' in the dateMonths useMemo with that constant; update the call sites
that use formatDateDayjs(debouncedStartDate, 'YYYYMM') /
formatDateDayjs(debouncedEndDate, 'YYYYMM') (refer to the dateMonths memo and
formatDateDayjs usage) so they import and use DATE_FORMATS.MONTH_KEY instead of
the hardcoded literal.
In `@admin-ui/app/utils/dayjsUtils.ts`:
- Around line 11-17: DATE_FORMATS contains a TOKEN_DATETIME using the
nonstandard 'YYYY/DD/MM HH:mm:ss' order which may confuse future maintainers;
add a brief inline comment next to the TOKEN_DATETIME entry in the DATE_FORMATS
constant explaining that this specific day-before-month format is intentional
and required for the ClientActiveTokens component (mentioning ClientActiveTokens
and TOKEN_DATETIME) so readers know it must not be changed.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@admin-ui/plugins/auth-server/components/Scopes/ScopeForm.tsx`:
- Around line 674-678: Replace the duplicated literal date format in the
ScopeForm display with the centralized constant: change the call to
formatDate(scope.creationDate, 'YYYY-MM-DD HH:mm:ss') to use
DATE_FORMATS.DATETIME_SECONDS instead; update the Input defaultValue expression
where formatDate is used so it references DATE_FORMATS.DATETIME_SECONDS (ensure
DATE_FORMATS is imported if not already).
- Around line 607-611: Replace the hard-coded date format string in the
ScopeForm component with the centralized constant: change the call using
formatDate(scope.creationDate, 'YYYY-MM-DD HH:mm:ss') to use
DATE_FORMATS.DATETIME_SECONDS; update the import if necessary so DATE_FORMATS is
available in ScopeForm.tsx and ensure the Input defaultValue uses
formatDate(scope.creationDate, DATE_FORMATS.DATETIME_SECONDS) to keep formatting
consistent across the app.
|
duttarnab
approved these changes
Jan 30, 2026
moabu
approved these changes
Jan 30, 2026
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
4 participants
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.
fix(admin-ui): remove moment.js and standardize date handling with dayjs (#2577)
Summary
The Admin UI was using both
moment.jsanddayjsfor date and time handling, which is redundant and increases bundle size unnecessarily. This PR removesmoment.jsand standardizes all date-related functionality ondayjs.Problem
moment.jsanddayjswere being used for the same purpose.moment.jshas a larger bundle size (~23 KB gzipped) and slower load time compared todayjs.Fix Details
moment.jsdependency from the project.dayjs.Result
Additional Notes
This change aligns with modern frontend best practices and prepares the Admin UI for improved performance and maintainability.
🔗 Ticket
Closes: #2577
Summary by CodeRabbit
New Features
Refactor
Chores
Style
Tests
✏️ Tip: You can customize this high-level summary in your review settings.