-
Notifications
You must be signed in to change notification settings - Fork 72
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
feat: improve payment network types #1355
Conversation
if (!creationParameters.acceptedTokens) { | ||
throw Error('acceptedTokens is required'); | ||
} | ||
if (creationParameters.acceptedTokens.length === 0) { | ||
throw Error('acceptedTokens cannot be empty'); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a change in error message to make it clearer
@@ -7,7 +7,7 @@ import { generate8randomBytes } from '@requestnetwork/utils'; | |||
* Abstract class to extend to get the payment balance of conversion requests | |||
*/ | |||
export abstract class AnyToAnyDetector< | |||
TExtension extends ExtensionTypes.PnFeeReferenceBased.IFeeReferenceBased, | |||
TExtension extends ExtensionTypes.PnFeeReferenceBased.IFeeReferenceBased<any>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there are a few any
introduced in the type constraints. I think they are fine, as they are only on abstract classes, and concrete implementations enforce a type.
network: EvmChainName; | ||
acceptedTokens: string[]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
match what's expected by the PN! 🎉
@@ -51,17 +46,27 @@ export interface IPaymentNetworkCreateParameters<T = any> { | |||
|
|||
export type PaymentNetworkCreateParameters = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the "complexity" of the type prevented usage of Extract
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Arg, I remembered struggling with this.
paymentNetworkCreationParameters: TCreationParameters, | ||
) => Promise<ExtensionTypes.IAction<any>>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
slightly more precise...
|
||
/** Parameters for the creation action */ | ||
export type ICreationParameters = PnAnyToAnyConversion.ICreationParameters; | ||
export type ICreationParameters = Omit<PnAnyToAnyConversion.ICreationParameters, 'network'> & { | ||
network: ChainName; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
network is mandatory for AnyToEth
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Long-time expected
@@ -51,17 +46,27 @@ export interface IPaymentNetworkCreateParameters<T = any> { | |||
|
|||
export type PaymentNetworkCreateParameters = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Arg, I remembered struggling with this.
2c5f8fb
to
1381b85
Compare
1381b85
to
0bdf46f
Compare
Nice 👍 |
Make payment network parameters strongly typed, and more precise:
Example