Skip to content

refactor(deprecation): replace moment with luxon#13207

Merged
gkartalis merged 7 commits intomainfrom
gkartalis/replace-moment-with-luxon
Mar 12, 2026
Merged

refactor(deprecation): replace moment with luxon#13207
gkartalis merged 7 commits intomainfrom
gkartalis/replace-moment-with-luxon

Conversation

@gkartalis
Copy link
Member

This PR resolves []

Description

Completely removes moment and moment-timezone and replaces them with luxon.

Migration was performed along with Claude so please review carefully.

Followups:

  • thorough code review
  • test out functionality

PR Checklist

  • I have tested my changes on the following platforms:
    • Android.
    • iOS.
  • I hid my changes behind a feature flag, or they don't need one.
  • I have included screenshots or videos at least on Android, or I have not changed the UI.
  • I have added tests, or my changes don't require any.
  • I added an app state migration, or my changes do not require one.
  • I have documented any follow-up work that this PR will require, or it does not require any.
  • I have added a changelog entry below, or my changes do not require one.

To the reviewers 👀

  • I would like at least one of the reviewers to run this PR on the simulator or device.
Changelog updates

Changelog updates

Cross-platform user-facing changes

iOS user-facing changes

Android user-facing changes

Dev changes

  • replace moment with luxon

Need help with something? Have a look at our docs, or get in touch with us.

@gkartalis gkartalis self-assigned this Jan 30, 2026
@gkartalis gkartalis requested review from MounirDhahri, MrSltun and brainbicycle and removed request for MounirDhahri January 30, 2026 15:59
@gkartalis gkartalis added the FF label Jan 30, 2026
Copy link
Member

@MounirDhahri MounirDhahri left a comment

Choose a reason for hiding this comment

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

finally happened 😄 😄 😄

MounirDhahri
MounirDhahri previously approved these changes Jan 30, 2026
@gkartalis
Copy link
Member Author

@MounirDhahri yessss but we need to carefully review this PR in case claude missed any spots haha

Copy link
Member

@MounirDhahri MounirDhahri left a comment

Choose a reason for hiding this comment

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

Here is the output of me asking Opus to review this

Issues to Address

  1. Test Behavior Change (AuctionResultListItem.tests.tsx:223-225)
  • expect(navigate).not.toHaveBeenCalled()
  • expect(navigate).toHaveBeenCalled()
    This test assertion was inverted. The original test expected that when onPress is specified, navigate should NOT be called. The
    new test expects the opposite. This appears unrelated to the moment→luxon migration and needs verification.
  1. Ordinal Date Format Loss (TimeSince.tsx:22)
  • return date.format("ddd, MMM Do, h:mm A")
  • return date.toFormat("EEE, MMM d, h:mm A")
    Moment's Do produces ordinal numbers ("1st", "2nd", "3rd") while Luxon's d produces plain numbers ("1", "2", "3"). This changes
    output from "Wed, Jan 1st, 3:30 PM" to "Wed, Jan 1, 3:30 PM". If ordinals are desired, you'll need a custom helper.
  1. Describe Block Mistakenly Converted to it (Timer.tests.tsx:170)
  • describe("displays the minutes when the sale does not end on the hour", () => {
  • it("displays the minutes when the sale does not end on the hour", () => {
    A describe block was changed to an it block. This removes the grouping structure. If intentional, the block body should be
    reviewed for nested tests.
  1. Potential Null Timezone (Timer.tsx:119)

const userZone = DateTime.local().zoneName
zoneName can return null in rare edge cases. Consider:
const userZone = DateTime.local().zoneName ?? "UTC"

Files Needing Extra Attention

  • src/app/Components/Lists/tests/AuctionResultListItem.tests.tsx - behavior change
  • src/app/Scenes/Inbox/Components/Conversations/TimeSince.tsx - ordinal format change
  • src/app/Components/Bidding/Components/tests/Timer.tests.tsx - describe→it change
  • src/app/utils/saleTime.ts - complex timezone logic, highest risk

@MounirDhahri
Copy link
Member

I reviewed Claude's output, and it seems to be valid in this case

@gkartalis
Copy link
Member Author

fixing the tests and will review afterwards

@gkartalis gkartalis marked this pull request as draft January 30, 2026 16:46
@artsy artsy deleted a comment from ArtsyOpenSource Jan 30, 2026
@github-actions
Copy link

github-actions bot commented Feb 3, 2026

🎉 Beta Versions Generated (commit: a639097)

Android 🤖

  • 8.93.0 (2026020309) - Available on Play Store
  • 8.93.0 (2026020309) - Available on Firebase

iOS 🍏

  • 8.93.0 (2026.02.03.09) - Available on TestFlight
  • 8.93.0 (2026.02.03.09) - Available on Firebase

@gkartalis gkartalis marked this pull request as ready for review February 9, 2026 16:56
MrSltun
MrSltun previously approved these changes Feb 11, 2026
Copy link
Member

@MrSltun MrSltun left a comment

Choose a reason for hiding this comment

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

Thank you for this change, finally!!
I left a few comments, but other than that it looks great! 🌟

Comment on lines +80 to +82
.toFormat("h:mma ZZZZ")
.replace("AM", "am")
.replace("PM", "pm")
Copy link
Member

Choose a reason for hiding this comment

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

Suggestion: I see that this is being used in multiple places. Should we move it to a util file?


describe("absolute date label", () => {
it("shows the start date if the sale has not started", () => {
it("shows the start date if the sale has not started", async () => {
Copy link
Member

Choose a reason for hiding this comment

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

👀

@gkartalis gkartalis force-pushed the gkartalis/replace-moment-with-luxon branch from a639097 to 641f867 Compare February 13, 2026 10:29
@artsy artsy deleted a comment from ArtsyOpenSource Feb 13, 2026
@gkartalis gkartalis force-pushed the gkartalis/replace-moment-with-luxon branch from 1dc8e67 to 8fb7641 Compare March 2, 2026 13:41
@artsy artsy deleted a comment from ArtsyOpenSource Mar 2, 2026
@artsy artsy deleted a comment from ArtsyOpenSource Mar 2, 2026
@github-actions
Copy link

github-actions bot commented Mar 2, 2026

🎉 Beta Versions Generated (commit: 66c59eb)

iOS 🍏

  • 9.0.0 (2026.03.02.14) - Available on Firebase

Android 🤖

  • 9.0.0 (2026030214) - Available on Firebase
  • 9.0.0 (2026030214) - Available on Play Store

gkartalis and others added 4 commits March 10, 2026 17:43
Replace all moment.js and moment-timezone usage with Luxon equivalents:
- Update test infrastructure to support Luxon timezone mocking
- Migrate duration components to use Luxon Duration API
- Convert complex timezone conversion logic in saleTime.ts
- Update timer and countdown components
- Migrate date formatting, comparisons, and arithmetic operations
- Remove moment.js and moment-timezone dependencies

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@gkartalis gkartalis force-pushed the gkartalis/replace-moment-with-luxon branch from 66c59eb to 66a0cfa Compare March 10, 2026 16:58
@artsy artsy deleted a comment from ArtsyOpenSource Mar 10, 2026
@artsy artsy deleted a comment from ArtsyOpenSource Mar 11, 2026
hours: duration.hours(),
minutes: duration.minutes(),
seconds: duration.seconds(),
days: duration.as("days"),
Copy link
Member Author

Choose a reason for hiding this comment

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

Before After
Image Image

Comment on lines 125 to +268
@@ -254,9 +259,13 @@ export const AuctionResult: React.FC<Props> = (props) => {
<Text variant="sm-display">{auctionResult.title}</Text>
<Text variant="xs" color="mono60" my={0.5}>
{[
moment(auctionResult.saleDate).utc().format("MMM D, YYYY"),
auctionResult.saleDate
? DateTime.fromISO(auctionResult.saleDate).toUTC().toFormat("MMM d, yyyy")
: "",
auctionResult.organization,
].join(" • ")}
]
.filter(Boolean)
.join(" • ")}
Copy link
Member Author

Choose a reason for hiding this comment

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

Before After
Image Image

Comment on lines +23 to +32
DateTime.fromISO(endAt).diff(DateTime.fromISO(currentTime)).toISO() || "PT0S"
)
const daysTilEnd = durationTilEnd.as("days")
const hoursTillEnd = durationTilEnd.as("hours")
const secondsTilEnd = durationTilEnd.as("seconds")

const hasStarted =
Duration.fromISO(DateTime.fromISO(startAt).diff(DateTime.fromISO(currentTime)).toString())
.seconds < 0
Duration.fromISO(
DateTime.fromISO(startAt).diff(DateTime.fromISO(currentTime)).toISO() || "PT0S"
).seconds < 0
Copy link
Member Author

Choose a reason for hiding this comment

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

Before After
Image Image

@artsy artsy deleted a comment from ArtsyOpenSource Mar 11, 2026
@ArtsyOpenSource
Copy link
Contributor

This PR contains the following changes:

  • Dev changes (replace moment with luxon - gkartalis)

Generated by 🚫 dangerJS against 0ba7fba

@gkartalis gkartalis merged commit 6584717 into main Mar 12, 2026
9 checks passed
@gkartalis gkartalis deleted the gkartalis/replace-moment-with-luxon branch March 12, 2026 08:34
gkartalis added a commit that referenced this pull request Mar 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants