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

[bug] PaymentIntent type inconsistencies #856

Open
leggomuhgreggo opened this issue Nov 19, 2024 · 0 comments
Open

[bug] PaymentIntent type inconsistencies #856

leggomuhgreggo opened this issue Nov 19, 2024 · 0 comments
Labels
bug Something isn't working

Comments

@leggomuhgreggo
Copy link

leggomuhgreggo commented Nov 19, 2024

Overview

I needed to add a feature that generated a receipt, based on the stripe payment information, and ran into some issues with the type interface.

The most significant difficulty in my cases was the need to use the as unknown as technique with an override type, when consuming receipt data — primarily due to a pluralization inconsistency between terminalVerificationResult (expected) and terminalVerificationResults (actual)

Example Code
type ReceiptDetailsBase = PaymentIntent.Type['offlineDetails']['cardPresentDetails']['receiptDetails']; 

type ReceiptDetailsOverride = Omit<ReceiptDetailsBase, 'terminalVerificationResult'> & {
  terminalVerificationResults: ReceiptDetailsBase['terminalVerificationResult'];
} | undefined;

export function generateOrderReceipt(paymentIntent: PaymentIntent.Type) {
  const { paymentMethodDetails } = paymentIntent.charges[0];
  const { cardPresentDetails } = paymentMethodDetails;

  const receiptDetails = cardPresentDetails?.receipt as unknown as ReceiptDetailsOverride;
  ...
}    

Upon investigating further, there appear to be additional type mismatch issues (see below)

Repro

  1. Add console.log(JSON.stringify(paymentIntent)) in payment handler
  2. Submit test payment
  3. Copy log output and set as typed variable like const ExamplePaymentIntent: PaymentIntent.Type = {...}
  4. Observe TS errors
Screenshot

image

Additional Considerations

There's a lot one could do with type definitions

I think to start with, just ensuring that the types match what's being generated would be an excellent improvement.

Type Annotations?

Another great enhancement would be to add annotations to the types so that they can be picked up via intellisense.

Here's an example where it could be useful:

Our existing receipt interface included an emv object and it wasn't immediately obvious how to map these fields

  emv: {
    application_label: ???,
    network_label: ???,
    mode: ???,
    tvr: ???,
    arc: ???,
    ac: ???,
    tsi: ???,
    cvm: ???,
    aid: ???,
    iad: ???,
  }

Big fan of the decision to prefer verbose property names over abbreviation, but I think this is an area where type annotations could be really useful, as they could include the standard abbreviation for a given field.

Eventually I landed on this (although I think I need to iron out a couple things)

emv: {
  application_label: receiptDetails?.applicationPreferredName,
  network_label: cardPresentDetails?.network,
  mode: 'Issuer', // @REVIEW
  tvr: receiptDetails?.terminalVerificationResults, // @REVIEW
  arc: receiptDetails?.authorizationResponseCode,
  ac: receiptDetails?.applicationCryptogram,
  tsi: receiptDetails?.transactionStatusInformation,
  cvm: receiptDetails?.accountType,
  aid: receiptDetails?.dedicatedFileName,
  iad: receiptDetails?.applicationCryptogram,
},
Discriminated Unions?

I think in a perfect world the PaymentIntent type would be a union of every major permutation of the model, something like:

type PaymentIntent = PaymentIntentInitial | PaymentIntentConfirmed | PaymentIntentCancelled

Note: Since it's aggregating other models the unions, those nested, constituent models would also probably have union types

Ideally with this approach there's a dedicated enum field that can be used for the discriminated union pattern

https://www.typescriptlang.org/docs/handbook/2/narrowing.html#discriminated-unions

I think there's a way to include type narrowing via the return type, but it may also be advantageous to have a generic where users can pass the intended permutation as an argument to the type interface.

Consolidation?

I think there's also something to be said for consolidating fields, eg:
type CardPresentDetails = CardPresentDetailsStandard | InteracPresentDetails

It was sometimes difficult to know which selection path to use — and it would be great to be able to rely more decisively on the type signature and remove the need for trial and error.

Please let me know if I can be of any additional help!

Thanks!!

@leggomuhgreggo leggomuhgreggo changed the title Fix PaymentIntent type inconsistencies [bug] PaymentIntent type inconsistencies Nov 20, 2024
@xiaoshen-stripe xiaoshen-stripe added the bug Something isn't working label Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants