Adyen / adyen-web

Adyen Web Drop-in and Components
https://docs.adyen.com/online-payments
MIT License
174 stars 126 forks source link

Stronger TS types for event handlers #2682

Open brodyd795 opened 2 months ago

brodyd795 commented 2 months ago

Is your feature request related to a problem? Please describe.

The current TypeScript types for the @adyen/adyen-web package include some any types, e.g. in the onChange handler for the AdyenCheckout component. This makes for a less-than-ideal developer experience.

Describe the solution you'd like

I'd love to see stronger typing for this module.

Describe alternatives you've considered

We're currently creating home-grown types to work around this, but we'd prefer this to be native to the library.

Additional context

Here is an example of an any that I'd like to see typed more strongly.

ashrafnazar commented 2 months ago

I personally would like to see a TS declarations repo, i.e. @types/adyen

ribeiroguilherme commented 2 months ago

Hey @brodyd795,

Thanks for the feedback. The good news is that we are revamping the whole typescript types for the next major release, which should happen in the following weeks, and there are improvements for the any types.

Could you share a bit more the details in how are you using this 'home-grown types' ? Maybe a code snippet showing how you are narrowing down and differentiating types. That can bring some insights for us.


@ashrafnazar there is no intention to have a TS declaration repo, since the library is already typed and it also export the types.

ashrafnazar commented 2 months ago

@ribeiroguilherme - could this prove to be a problem where the moduleResolution property in .tsconfig files is Bundler?

ribeiroguilherme commented 2 months ago

@ashrafnazar I don't understand your point. Can you elaborate better the issue?

brunolopesr-dft commented 2 months ago

@ribeiroguilherme I think @ashrafnazar refers to when we set the moduleResolution to bundler in .tsconfig.json file, it's a new type of moduleResolution that came with Typescript 5.

The problem with this type of moduleResolution, is that it only looks to the files exposed through the exports field of @adyen/adyen-web library. When moduleResolution is bundler, then we cannot import the types directly with relative paths, for example:

import SecuredFieldsElement from '@adyen/adyen-web/dist/types/components/SecuredFields'

This will gave a error, because the exports field of the package points to a ./dist/types/index.d.ts with this content:

import { CoreOptions } from './core/types';
import Checkout from './core';
declare function AdyenCheckout(props: CoreOptions): Promise<Checkout>;
export default AdyenCheckout;

This is ./dist/index.d.ts from @adyen/adyen-web@5.64.0, the unique type available to import is the constructor of AdyenCheckout

But this is not related to this issue at all, I think it's better related to #363 that will be closed when V6 lands.

ashrafnazar commented 2 months ago

Is there an ETA for v6?

brodyd795 commented 2 months ago

Could you share a bit more the details in how are you using this 'home-grown types' ? Maybe a code snippet showing how you are narrowing down and differentiating types. That can bring some insights for us.

@ribeiroguilherme Sure!

The AdyenCheckout configuration has an onChange handler that takes a single parameter of state, which is currently typed as any, so we've built this custom type until y'all get time to add stricter types for it.


type AdyenPaymentData = {
  paymentMethod: {
    type: string;
    checkoutAttemptId: string;
  };
  // ...etc
};

type AdyenPaymentState = {
  data: AdyenPaymentData;
  errors: Record<string, unknown>;
  valid: Record<string, unknown>;
  isValid: boolean;
};
ribeiroguilherme commented 1 month ago

@brodyd795 thanks for sharing!

You can already preview it here . We can provide types for most of the properties, except the ones available inside the paymentMethod field, as they are dynamic based on the payment type.

Out of curiosity - why are you using the onChange handler and not relying on the onSubmit?

@ashrafnazar - we are aiming to release the beta version this month along with the migration guide/release notes. We will announce it on this thread . Feel free to subscribe for updates