Skip to content

chore: fix main TypeScript baseline#346

Merged
islandbitcoin merged 2 commits into
mainfrom
fix/main-test-typecheck
May 11, 2026
Merged

chore: fix main TypeScript baseline#346
islandbitcoin merged 2 commits into
mainfrom
fix/main-test-typecheck

Conversation

@forge0x
Copy link
Copy Markdown
Contributor

@forge0x forge0x commented May 11, 2026

Summary

Fixes the current main TypeScript baseline by removing stale Galoy-only test suites and tightening the remaining Flash-facing types instead of keeping compatibility shims for unused tests.

Highlights:

  • Removes unused test/galoy executable test suites (bats, e2e, integration, legacy-integration, unit)
  • Keeps shared test/galoy/helpers/mocks that are still imported by test/flash
  • Removes src/services/kratos/tests-but-not-prod.ts, which was only referenced by the removed Galoy e2e tests
  • Removes the unused legacy PaymentInputValidator compatibility export and related types
  • Replaces invoice amount: number compatibility types with branded domain amounts (FractionalCentAmount / BtcPaymentAmount)

Validation

Run with Node 20.20.0:

  • yarn tsc-check-noimplicitany
  • yarn tsc-check
  • yarn build
  • Changed-file ESLint ✅
  • Changed-file Prettier ✅

Note: local full yarn eslint-check from nested worktrees can be blocked by duplicate ESLint plugin resolution between the worktree and parent checkout; changed files pass ESLint with plugins resolved from this worktree.

@forge0x forge0x force-pushed the fix/main-test-typecheck branch from aac04bd to d70603a Compare May 11, 2026 11:36
Copy link
Copy Markdown
Contributor

@brh28 brh28 left a comment

Choose a reason for hiding this comment

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

the tests in tests/galoy are unused. They were still lingering just as a reference to be migrated into tests/flash overtime. If there causing any issues, I'd be favor of just removing them

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

strange file. I'm curious what this is used for

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

no idea. kratos is still black-box-ish to me

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed this file. It was only referenced by the deleted Galoy e2e tests, so keeping it as a production-adjacent test helper was unnecessary.

Comment thread src/app/wallets/index.types.d.ts Outdated
type AddInvoiceForSelfArgs = {
walletId: WalletId
amount: FractionalCentAmount // only supports USD for now
amount: number | FractionalCentAmount // only supports USD for now
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

typically don't want to be handling javascript number types

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

whats a better way to fix the type check error here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Replaced the JS number compatibility amounts with branded domain types: FractionalCentAmount for USD invoice amounts and BtcPaymentAmount for BTC compatibility stubs. The remaining helper call sites now convert explicitly at the test boundary.

checkOnchainMin,
]) No newline at end of file
])
export const PaymentInputValidator = (getWalletFn: PaymentInputValidatorConfig) => ({
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this appears to be unused

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed the unused legacy PaymentInputValidator export and its declaration types. Its only remaining consumers were in the deleted Galoy unit tests.

@bobodread876
Copy link
Copy Markdown
Collaborator

the tests in tests/galoy are unused. They were still lingering just as a reference to be migrated into tests/flash overtime. If there causing any issues, I'd be favor of just removing them

maybe we should just remove them all then... I'm in favor of it too.

@forge0x
Copy link
Copy Markdown
Contributor Author

forge0x commented May 11, 2026

Addressed review feedback in dd192e423:

  • Removed unused Galoy executable test suites while keeping test/galoy/helpers and test/galoy/mocks because test/flash still imports them.
  • Removed src/services/kratos/tests-but-not-prod.ts; it was only referenced by deleted Galoy e2e tests.
  • Removed unused legacy PaymentInputValidator compatibility export/types.
  • Replaced invoice amount: number compatibility types with branded domain amounts.

Verification: yarn tsc-check-noimplicitany, yarn tsc-check, yarn build, changed-file ESLint, and changed-file Prettier pass.

Copy link
Copy Markdown
Contributor

@brh28 brh28 left a comment

Choose a reason for hiding this comment

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

I'm going to update the commit message to chore: as the changes do change production code, even if minor

@brh28 brh28 changed the title test: fix main TypeScript baseline chore: fix main TypeScript baseline May 11, 2026
@islandbitcoin islandbitcoin merged commit 7c3f7e4 into main May 11, 2026
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.

4 participants