Feat: Update Interface Types #402
Conversation
| * | ||
| * @see https://docs.mollie.com/reference/v2/payments-api/create-payment?path=billingAddress#credit-card | ||
| */ | ||
| billingAddress?: Address; |
There was a problem hiding this comment.
this is not actually removed, but picked from data (see above)
| /** | ||
| * The card token you got from Mollie Components. The token contains the card information (such as card holder, card number, and expiry date) needed to complete the payment. | ||
| * | ||
| * @see https://docs.mollie.com/reference/v2/payments-api/create-payment?path=cardToken#credit-card | ||
| */ | ||
| cardToken?: string; | ||
| shippingAddress?: Address & { |
There was a problem hiding this comment.
this is not actually removed, but picked from data (see above)
| * | ||
| * @see https://docs.mollie.com/overview/common-data-types?path=email#address-object | ||
| */ | ||
| email?: string; |
There was a problem hiding this comment.
@fjbender email is missing from the common type definition, but I think that's a mistake, as it is included where the address is used (payments/orders)
There was a problem hiding this comment.
It's likely a documentation bug as the common type definition is not auto-generated.
| @@ -102,7 +120,13 @@ export interface Address { | |||
| * | |||
| * @see https://docs.mollie.com/overview/common-data-types?path=postalCode#address-object | |||
| */ | |||
| postalCode: string; | |||
| postalCode?: string; | |||
There was a problem hiding this comment.
this is the only potentially breaking change, as for this was used for orders and is actually optional.
This means users, who consumed order.billingAddress.postalCode may now get a type error.
There was a problem hiding this comment.
I agree that this particular change is more fitting for a bugfix version bump, than a major, because the original type definition (required instead of optional) was actually wrong. So if consumers of this library rely on postalCode not being optional, that could actually mean runtime errors on their end.
Forcing them to fix TS errors due to postalCode being optional is actually a good thing for all parties involved, IMO. So my vote too goes to not warranting a major version bump.
janpaepke
added
the
API discrepancy
There was a problem hiding this comment.
LGTM from a code standpoint. @fjbender I know you synced with @janpaepke in terms of inconsistencies between actual API and API documentation. Would you mind giving this PR a once-over to confirm that these changes are in fact in line with whatever you discussed with Jan?
This resolve several type discrepancies between the client and the mollie API.
Since the official docs are too inconsistent to be used as source of truth, the actual optional-ness of these properties was aligned directly with mollie.
No properties were removed and Types should be wider, not narrower, making this a backwards compatible change.
The only exception I could find was the
address.postCode, which was defined to be required, when it is actually optional, since not all countries have a post code.This change will concern the organization.address and order.address.
I don't believe this warrants a major version release.
This resolves:
Supersedes #397