bitpay / nodejs-bitpay-client

NodeJs implementation for the BitPay Cryptographically Secure RESTful API
MIT License
48 stars 42 forks source link

Typescript Type mismatches #94

Closed asymptotik closed 2 months ago

asymptotik commented 11 months ago

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 commented 10 months ago

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

bobbrodie commented 9 months ago

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!

bobbrodie commented 2 months ago

Hi! We've done a lot of work implementing Zod and checking types with the API team. It's been out for a bit, so I'll go ahead and close this. Please feel free to reach out if you have any other issues. Thanks!