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

Stronger TS types for event handlers #2682

Open
brodyd795 opened this issue May 14, 2024 · 8 comments
Open

Stronger TS types for event handlers #2682

brodyd795 opened this issue May 14, 2024 · 8 comments
Labels
Enhancement New feature or request v6 Included in the next major release

Comments

@brodyd795
Copy link

brodyd795 commented May 14, 2024

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.

@brodyd795 brodyd795 added the Enhancement New feature or request label May 14, 2024
@ashrafnazar
Copy link

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

@ribeiroguilherme
Copy link
Contributor

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.

@ribeiroguilherme ribeiroguilherme added v6 Included in the next major release and removed To be investigated labels May 24, 2024
@ashrafnazar
Copy link

ashrafnazar commented May 24, 2024

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

@ribeiroguilherme
Copy link
Contributor

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

@brunolopesr-dft
Copy link

brunolopesr-dft commented May 24, 2024

@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/[email protected], 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
Copy link

Is there an ETA for v6?

@brodyd795
Copy link
Author

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
Copy link
Contributor

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request v6 Included in the next major release
Projects
None yet
Development

No branches or pull requests

5 participants