Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Typescript Type mismatches #94

Open
asymptotik opened this issue Oct 15, 2023 · 2 comments
Open

Typescript Type mismatches #94

asymptotik opened this issue Oct 15, 2023 · 2 comments

Comments

@asymptotik
Copy link

asymptotik commented Oct 15, 2023

Maybe I'm missing something, but there seems to be a lot of typescript types that do not match the data. Here are some examples with Invoice. I get an invoice back from a call to create an invoice and I get back something with a fragment like this. (I'm paring things way back for brevity)

{
  "exchangeRates": {
      "BTC": {
        "USD": 27008.379999999997,
        "BCH": 125.84865570103908,
        "ETH": 17.3262809450799,
        "GUSD": 27008.379999999997
     }
  }
}

Then we have the InvoiceInterface.

export interface InvoiceInterface {
   exchangeRates: Array<[string, Array<[string, number]>]> | null;
}

Shouldn't this be something more like:

export interface InvoiceInterface {
   exchangeRates: Record<string, Record<string, number>> | null;
}

You can do something like this to see the incompatability:

const exchangeRates = {
  "BTC": {
    "USD": 27008.379999999997,
    "BCH": 125.84865570103908,
    "ETH": 17.3262809450799,
    "GUSD": 27008.379999999997
 }
}

// Type '{ BTC: { USD: number; BCH: number; ETH: number; GUSD: number; }; }' is missing the following properties from type 
// '[string, [string, number][]][]': length, pop, push, concat, and 28 more.ts(2740)
const p: InvoiceInterface['exchangeRates'] = exchangeRates

// works
const p2: Record<string, Record<string, number>> = exchangeRates

There are quite a few of these (paymentSubtotals, paymentTotals, paymentDisplayTotals, MinerFees, etc) along with quite a few other other typing issues. This makes it very difficult to use this library with typescript. What I ended up doing was making my own copy of the library and updating the types where I need them. Would really great to see some of this get some attention. Or, maybe I'm just missing something. Happens all the time :).

@bobbrodie
Copy link
Contributor

Hey @asymptotik thanks for bringing this to our attention -- we're checking it out and will report back :)

@bobbrodie
Copy link
Contributor

Hey @asymptotik I just wanted to drop a quick update on this. On our first major revision in a while, we were focused on not having too much disruption while we brought in tests for existing code, and now we’re reviewing as a bigger group, making sure there’s alignment with the latest API and considering runtime type checks.

We are comparing the TS interfaces to each API response through the functional tests, using Zod to easily catch mismatches (in addition to manual review), and are updating accordingly for the 6.0 release.

We've recently been discussing how we're going to handle releases going forward, and are formalizing a release plan that we'll publicize, to better align with upstream language and framework releases.

Also, I realized that I didn't happen to receive an alert on this, so we're reviewing and updating settings.

Thanks!

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

No branches or pull requests

2 participants