Skip to content

[Due for payment 2026-10-12] [$250] [Refactor] Refactor TransactionUtils/index.ts to stay under the 4000-line ESLint limit #102012

Closed

Description

@MariaHCD

Problem

src/libs/TransactionUtils/index.ts grew to 4001 lines via #101324.

Our ESLint config caps files in src/ at 4000 lines:

'max-lines': ['error', 4000],

We disabled the rule in order to move the PR forward but that was a temporarily solution.

Solution

Refactor to separate the file into smaller utils

Issue OwnerCurrent Issue Owner: @mallenexpensify
Upwork Automation - Do Not Edit

Activity

  1. MariaHCD commented on Sep 23, 2026

    @MariaHCD
    ContributorAuthor

    @MelvinBot review this issue and concisely outline the technical plan. Keep language simple, succinct and human-readable.

  2. self-assigned this
    on Sep 23, 2026
  3. MelvinBot commented on Sep 23, 2026

    @MelvinBot
    Contributor

    Technical plan

    src/libs/TransactionUtils/index.ts is sitting at exactly 4000 lines, which is the cap set in config/eslint/eslint.config.mjs:739. So any new line in that file fails lint today — not just the one from Hide duplicate violation and Review duplicates action on IOU reports.

    The fix: move each group of functions into its own sibling file under src/libs/TransactionUtils/, and leave index.ts as a pure re-export barrel. 215 files import from @libs/TransactionUtils (154 with named imports), and none of them need to change.

    Proposed files

    New file What moves Current lines ~Size
    requestType.ts isDistanceRequest → isScanningTransaction (predicates, getRequestType, getTransactionType, translation keys) 164–458 ~295
    buildOptimisticTransaction.ts buildOptimisticTransaction 459–624 ~165
    getters.ts receipt/field presence checks + all getX field readers (getAmount, getCurrency, getMerchant, attendees, MCC, tags, dates) 625–730, 1287–1779 ~600
    getUpdatedTransaction.ts getClearedPendingFields, distance-merchant helpers, getUpdatedTransaction 731–1286 ~555
    cardTransactions.ts Expensify/managed card helpers, pending-card logic, delete prompts, block modals 1780–1962 ~185
    scanning.ts isScanning → hasMissingSmartscanFields 1963–2044 ~80
    violations.ts getTransactionViolations → hasPendingUI, isDuplicate, isViolationDismissed, hasViolation, notice/warning types, plus submission-blocking helpers (3621–3700) 2045–2385, 2477–2683, 3621–3700 ~630
    waypoints.ts hasRoute → getValidWaypoints 2386–2466 ~80
    tax.ts calculateTaxAmount → getTaxRateTitle, getCategoryTaxDetails 2684–2932, 3352–3372 ~270
    duplicates.ts getValidDuplicateTransactionIDs → buildMergeDuplicatesParams 2934–3351 ~420
    splits.ts isExpenseSplit → shouldRedirectDeleteToSplitExpenseEdit 3387–3457 ~70
    index.ts nothing but imports + the existing export {…} block — ~250

    The one rule that makes this work

    index.ts must end up with zero logic. If it keeps even a few helpers, the new modules will need to import from ./index while index imports them back — a circular dependency. Instead, modules import each other directly and only "downward":

    requestType → getters → {waypoints, tax, cardTransactions, scanning, violations} → {duplicates, splits, getUpdatedTransaction, buildOptimisticTransaction}
                                                                                                        ↓
                                                                                                  index.ts (barrel)
    

    Nothing imports index.ts.

    Two gotchas

    1. violations.ts and duplicates.ts will fight. isDuplicate calls shouldShowViolation and isViolationDismissed, and hasDuplicateTransactions calls isDuplicate. Keep all four in violations.ts and limit duplicates.ts to the merge/compare flow (getValidDuplicateTransactionIDs, compareDuplicateTransactionFields, buildMergeDuplicatesParams). That keeps the edge one-directional.
    2. Don't let consumers import the new submodules directly. About 9 test files do jest.mock('@libs/TransactionUtils', () => ({…})). Those keep working as long as callers import from the barrel. The moment a component imports @libs/TransactionUtils/violations, that mock silently stops applying and the test passes for the wrong reason.

    Suggested PR order

    Split this into small PRs rather than one 4000-line diff — a single PR touching everything is near-impossible to review and will conflict with every open transaction PR.

    1. Unblock first: extract getUpdatedTransaction.ts + buildOptimisticTransaction.ts (~720 lines, self-contained, almost no internal callers). That alone drops index.ts to ~3300 and clears the lint failure.
    2. duplicates.ts + tax.ts (~690 lines).
    3. violations.ts (~630 lines) — the riskiest, do it alone.
    4. Everything else, finishing with emptying index.ts into a pure barrel.

    Each PR should be a pure move: cut, paste, fix imports, add to the export block. No renames, no signature changes, no "while I'm here" cleanups — that's what keeps review cheap.

    How to verify

    npm run lint       # confirms max-lines passes
    npm run typecheck
    npx jest tests/unit/TransactionUtilsTest.ts

    Also re-run the ~15 test files that touch TransactionUtils (MergeTransactionUtilsTest, ViolationUtilsTest, useTransactionViolationsTest, TransactionPreviewUtils.test, etc.), since those are where a bad import edge shows up first.

    Note: tests/unit/TransactionUtilsTest.ts is 5980 lines, but the max-lines rule only covers src/**, so the test file doesn't have to be split. Splitting it to match the new modules would be nice, but it's optional and shouldn't ride along in these PRs.

    Why not just add /* eslint-disable max-lines */

    Seven files already do this (ReportUtils.ts, SearchUIUtils.ts, ROUTES.ts, styles/index.ts, ReportActionsUtils.ts, actions/Policy/Policy.ts, actions/Report/index.ts), so there's precedent and it would take 30 seconds. But it just moves the cap, and this file is one of the most-edited in the repo (215 importers). The barrel refactor is what actually stops it recurring.

    I reviewed this statically from the code — I couldn't verify anything in a browser on this run, though for a pure-move refactor lint, typecheck, and the unit suites are the real signal anyway.


    Next Steps for Contributor+ team:
    To accept: @MelvinBot implement [step 1 of this plan](https://github.com/Expensify/App/issues/102012) to create a draft PR.
    To refine: @MelvinBot <your feedback>
    To reject: Explain why you are rejecting Melvin's proposal.


    view run

  4. added
    ExternalAdded to denote the issue can be worked on by a contributor
    on Sep 23, 2026
  5. melvin-bot commented on Sep 23, 2026

    @melvin-bot

    Triggered auto assignment to Contributor-plus team member for initial proposal review - @ahmedGaber93 (External)

  6. melvin-bot commented on Sep 23, 2026

    @melvin-bot

    Unable to auto-create job on Upwork. The BZ team member should create it manually for this issue.

  7. ashusatyarthi-wq commented on Sep 27, 2026

    @ashusatyarthi-wq

    Proposal

    Please re-state the problem that we are trying to solve in this issue.

    src/libs/TransactionUtils/index.ts has grown to over 4,000 lines, violating the project's ESLint max-lines rule (config/eslint/eslint.config.mjs:739) which caps files in src/ at 4,000 lines. Any new addition to TransactionUtils currently fails the lint check.


    What is the root cause of that problem?

    Over time, utility functions for transaction creation, tax calculations, waypoints, smart scanning, card operations, duplicate management, violations, and report updates were all centralized into a single src/libs/TransactionUtils/index.ts monolithic file instead of domain-specific sibling modules.


    What changes do you think we should make in order to solve the problem?

    Refactor src/libs/TransactionUtils/ by extracting distinct functional domains into focused sibling files, while preserving index.ts as a pure re-export barrel.

    Because over 200 files in the codebase import from @libs/TransactionUtils, keeping index.ts as a complete re-export barrel ensures zero breaking changes and zero import updates needed across consumers.

    Proposed Module Breakdown:

    1. buildOptimisticTransaction.ts & getUpdatedTransaction.ts:
      • Extract buildOptimisticTransaction, getUpdatedTransaction, and getClearedPendingFields (~720 lines).
      • This single step immediately reduces index.ts down to ~3,280 lines, fully resolving the ESLint violation.
    2. tax.ts & waypoints.ts:
      • Extract tax calculation helpers (calculateTaxAmount, getTaxRateTitle, getDefaultTaxCode) and route/waypoint utilities (~350 lines).
    3. cardTransactions.ts & scanning.ts:
      • Extract card-specific helpers (isExpensifyCardTransaction, isPending, canDeleteCardTransactionByLiabilityType) and smartscan status checks (~265 lines).
    4. duplicates.ts:
      • Extract duplicate detection and merge parameter builders (~420 lines).
    5. index.ts (Barrel):
      • Contains strictly imports and re-exports with zero business logic, preventing circular dependency issues.

    Safety & Mock Integrity:

    • Consumers continue importing from @libs/TransactionUtils so all existing jest.mock('@libs/TransactionUtils', ...) mocks continue to function without silent failure.
    • Strictly pure moves: no renames, signature changes, or functional modifications in this refactor.

    What specific scenarios should be covered in tests?

    1. Lint & Typecheck:
      • npm run lint passes with 0 max-lines errors on src/libs/TransactionUtils/index.ts and all new submodules.
      • npm run typecheck passes with 0 errors.
    2. Unit Test Suite:
      • npx jest tests/unit/TransactionUtilsTest.ts passes completely.
      • Related test suites (MergeTransactionUtilsTest, ViolationUtilsTest, useTransactionViolationsTest, TransactionPreviewUtils.test) pass without regression.
  8. ahmedGaber93 commented on Sep 28, 2026

    @ahmedGaber93
    Contributor

    @MelvinBot implement [step 1 of this plan](#102012

  9. 37 remaining items

  10. melvin-bot commented on Oct 11, 2026

    @melvin-bot

    Payment Summary

    Resolving PRs:

    BugZero Checklist (@mallenexpensify)

    • I have confirmed assignees, roles, and Upwork contracts look correct
    • I have paid out Upwork contracts / manual NewDot requests
    • [BugZero Assignee] I have created a GH issue for creating/updating the regression test once above steps have been agreed upon
  11. MariaHCD commented on Oct 12, 2026

    @MariaHCD
    ContributorAuthor

    @ahmedGaber93 do we have one more PR to do here?

  12. ahmedGaber93 commented on Oct 12, 2026

    @ahmedGaber93
    Contributor
    1. Everything else, finishing with emptying index.ts into a pure barrel.

    @MariaHCD There is still one step left in Melvin's proposal, but I am not sure we need it on this issue. index.ts is 2013 lines, so it is already well under the 4000-line cap. We could empty it and turn it into a pure re-export file, but I am not sure we should follow this pattern. Wdyt?

  13. MariaHCD commented on Oct 12, 2026

    @MariaHCD
    ContributorAuthor

    Fair, I think it's fine for now 👍🏼

    Given that this was a relatively straightforward refactor but testing was presumably the more time intensive task. I think $250 is a fair price for the 3 PRs. Thoughts @ahmedGaber93 ?

  14. ahmedGaber93 commented on Oct 12, 2026

    @ahmedGaber93
    Contributor

    Sounds good to me, thanks!

  15. ahmedGaber93 commented on Oct 12, 2026

    @ahmedGaber93
    Contributor

    Contributor+ Checklist:

    • [Contributor] The offending PR and associated issue have been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake.

      Link to the comment on the PR: N/A. this a refactor issue
      Link to the comment on the Issue: N/A. this a refactor issue

    • [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner.

      Link to discussion: N/A.

    • [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

    This is a refactor issue, so no new regression test is required here.

  16. ahmedGaber93 commented on Oct 12, 2026

    @ahmedGaber93
    Contributor

    @mallenexpensify, as mentioned in this comment, this issue is worth $250.

  17. changed the title [-][Due for payment 2026-10-12] [$175] [Refactor] Refactor `TransactionUtils/index.ts` to stay under the 4000-line ESLint limit[/-] [+][Due for payment 2026-10-12] [$250] [Refactor] Refactor `TransactionUtils/index.ts` to stay under the 4000-line ESLint limit[/+] on Oct 13, 2026
  18. melvin-bot commented on Oct 13, 2026

    @melvin-bot

    Upwork job price has been updated to $250

  19. mallenexpensify commented on Oct 14, 2026

    @mallenexpensify
    Contributor

    Payment Summary

    Contributor+: @ahmedGaber93 due $250 via NewDot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Awaiting PaymentAuto-added when associated PR is deployed to productionDailyKSv2ExternalAdded to denote the issue can be worked on by a contributorTask

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions