Use type instead of enum

[shwao]

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. 😅

[shwao]

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. 🤦‍♂️

[Pimm]

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.

[shwao]

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:

Screen.Recording.2022-07-21.at.16.46.22.mov

Versus enum (ignore that Finger-Muscle-Memory-Error):

Screen.Recording.2022-07-21.at.16.48.03.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.

[janpaepke]

Assuming this is resolved. Feel free to reopen, if not.

[janpaepke]

@Pimm, did you reopen this because the PaymentStatus PR hadn't been merged or did you want to continue the discussion about enums vs types here?

My two cents on the latter are this:

We COULD change the type definitions to be this:

{
  method: `${PaymentStatus}`
}

This effectively turns the enum into a string literal.
This means people could use the string literals, should they prefer them.
If someone prefers enums they could use those as well. We should still export them and you CAN assign an enum to a compatible string literal, just not the other way around. This would also be a backwards compatible change.

The one downside I can think of is that the existence of those enums is now less obvious to a consumer (especially someone new to the SDK), because they're not nudged towards them by their IDE.
As a result most users will end up using strings.

So this does come down to personal preference a bit, I guess.
I for one don't see why you would ever want to sprinkle strings around your code, when you can use enums, which are more contextual (can include jsdoc comments) and easier maintainable (find all references, rename, etc.).

[karladler]

I am also supporting the suggested solution, since there is this situation where we need to use the actual values instead of stripped TS enums.

See also: https://typescript.tv/best-practices/why-typescript-enums-are-dead/