mollie-api-node
mollie-api-node copied to clipboard
Use type instead of enum
Some things that could be a TypeScript type
are enum
which is quite developer unfriendly. Code completion and many checks just don't work.
The Problem
For example if you have a Payment
and want to check its status, the IDE (VS Code in my case) can't give you anything to autocomplete. The type check works though.
Here you should have the statuses as suggestions:
This is the case for every enum
including Locale
, PaymentMethod
etc.
While the missing suggestions are basically just an inconvenience (and missing out on fully utilizing TypeScript and the IDE) using the embed
option of the mollieClient.payments.get
method just does not work without using any
because of the enum
:
In contrast to the include
option which uses a type
:
Changing the PaymentEmbed
from an enum
to type
fixes this:
Suggested Changes
Looking at the source code it seems like this change could be easy.
Lets use the PaymentStatus
as an example.
The PaymentStatus
enum
from src/data/payments/data.ts:825
is used like an Object in src/data/payments/PaymentHelper.ts
to check against the payment status.
I would suggest to change src/data/payments/data.ts
from this:
export enum PaymentStatus {
open = 'open',
canceled = 'canceled',
pending = 'pending',
authorized = 'authorized',
expired = 'expired',
failed = 'failed',
paid = 'paid',
}
To this:
export const PAYMENT_STATUS = {
open: 'open',
canceled: 'canceled',
pending: 'pending',
authorized: 'authorized',
expired: 'expired',
failed: 'failed',
paid: 'paid',
};
export type PaymentStatus = keyof typeof PAYMENT_STATUS;
And then use it in src/data/payments/PaymentHelper.ts
like this:
public isOpen(this: PaymentData): boolean {
return this.status === PAYMENT_STATUS.open;
}
The PaymentStatus
is now a type that can be used without problems.
Conclusion
I don't know why you at Mollie use enums instead of types but looking at the code i don't see a good reason. Maybe i just didn't find it yet or maybe there is none. I just know that using the Mollie Node package would be better if they were types not enums.
If this change is welcome i would create a PR for that. But i would need some guidance regarding the API_KEY in the .env file (Just the regular API key or something special?) and regarding the amount of errors when running tsc --noEmit
(Do i have to configure something special?). But I also do not insist on doing it myself. 😅
Oh damn. I just realized it... You are supped to use the Enum. I still think its cumbersome and my idea is better but at least i can use embed
without any
now. 🤦♂️
Thanks for the issue.
You are ‒ as you found out on your own ‒ supposed to use the enum in your code when writing TypeScript:
mollieClient.payments.create({ method: PaymentMethod.ideal, … })
This makes code completion work, and eliminates the need for any
.
However, I personally do not necessarily favour enums over union types of string constants (e.g. 'ideal' | 'creditcard' | 'directdebit' | …
). Code completion works either way, and both are safe (in the sense of type safety). I also think in terms of readability or "code cleanness":
.create({ method: PaymentMethod.ideal, …
is not better or worse than:
.create({ method: 'ideal', …
(For the sake of completeness and to anyone reading this issue: if you're writing JavaScript, you can use either; the former is required when writing TypeScript.)
As I don't really have a personal preference, I decided against introducing new enums for new parts of the API. (You've stumbled upon this in the form of PaymentInclude
.) I did so because I figured we can always replace union types for enums if we decide that that's the direction in which we want to go; whereas the reverse would cause a breaking change.
This is also the reason I haven't seriously considered de-enumming PaymentMethod
: doing so would cause a breaking change.
I'll bump heads with some people about this and see what they have to say.
Regarding .env
: for the time being, you'll have to put your API key in there if you want to run tests locally. Just go cp .env.example .env
and fill it out.
Regarding the "amount of errors" you get when executing the TypeScript compiler, I would have to see those errors. I don't have enough information to help you at this point.
It just occurred to me while making coffee. (Pauses are great) I think i just haven't seen an example on https://docs.mollie.com/ that uses these enums and didn't think of it before.
Code completion works either way
Kind off. Union types work better and faster as you don't need to import anything for it. Just type a quote and all the possible values are displayed. Also hovering over a union type does show you the possible values while the enum just shows the name PaymentStatus
.
Union type:
https://user-images.githubusercontent.com/63549997/180244193-6663903e-2f3b-4f83-bed8-4767a2be6bee.mov
Versus enum (ignore that Finger-Muscle-Memory-Error):
https://user-images.githubusercontent.com/63549997/180244571-e5a05298-033d-473e-b278-5346b1083ece.mov
Also i think in most IDE themes strings are better visible than variables which makes the union type better visible (at least for me).
I also realized that changing PaymentStatus
from an enum to a type would require refactoring and that this could only happen in a major version.
Okay thanks for your answer, i I look forward to what your colleagues say. In the meantime ill just use those enums.