Improve typings for client/config objects creation

[catchergeese]

Hello,

After using the SDK for some time, I noticed there is an opportunity to improve currently exposed API so that it better reflects use cases.

Because this is structured similarly in multiple places, let's maybe focus on (defined on Client class):
https://github.com/Adyen/adyen-node-api-library/blob/develop/src/client.ts#L103

public setEnvironment(environment: Environment, liveEndpointUrlPrefix?: string): void {
where
type Environment = "LIVE" | "TEST";

Assuming we have an instance of Client class client already instantiated, there are two (allowed by documentation) ways to set it, namely:

client.setEnvironement("TEST");
and
client.setEnvironement("LIVE", "your-endpoint-prefix");

unfortunately, current typings also make
client.setEnvironment("TEST", "your-endpoint-prefix");
and
client.setEnvironment("LIVE"); valid.

While the first one is not only unlikely but also of minor importance (nothing more than small technical glitch here), the second one compiles and suggests that nothing is wrong until the project is run in live environment - and it fails there with error "Please provide your unique live url prefix on the setEnvironment() call on the Client or provide checkoutEndpoint in your config object.". Error is informative and actionable, that's very good! But it's only available on runtime on live environment.

My suggestion would be to change current invocations to something along the lines of:
public setEnvironment(option: { environment: "TEST"; } | { environment: "LIVE"; liveEndpointUrlPrefix: string }): void {

It's only slightly less convenient to write it this way but it makes it obvious on compile-time that liveEndpointUrlPrefix needs to be passed or it won't work.

I prepared a small self-contained example here: https://www.typescriptlang.org/play?#code/C4TwDgpgBAKhDOwCiA7AbgSwE4HsUFsIVgoBeKAIhiQGUYKAoUSKAGQzQlU1wKJPIVWASQBqSRk3DRu2PIWJlYCZOjl9FAHzYcua3guAMpLAMIAbDPwAKAQyy3CwCFngB5TlnM5bAEyUA3gxQIVBEPPL8AFxQsgb8waFEvmA4GMQA-DGIWOkA5gwAvsbM0BZWxHYOTi7unt5+AEyBiSHh6oYxcIhxkcRFUNpBoWH6fcAx7Jy9GkYjyanpE1A5+QDcRcYMAMZ4iFC7KABmGHkA+pacMeU29o4Qzq4eLg3+5MNJY7MxQmISmzs9iRDidzs5ENdLLdqg9as8vD43lAPm0vp1KNQ6IxigwAPS4qCmHD4MAYczQFy4LBQAAWLmgAFooHkcDhfABCQEofYg06NC66SEVYBVe6POovRHNd6tUYRb6UETibHGQ48vCg-ng5Y3Sp3GpPepSlrzNHRDG0ehFIA

I'm happy to hear what are your views on the matter.

Cheers,
Michał

[KadoBOT]

Hi Michal,

Thanks for your suggestion! I'm 100% with you and would be happy to have this implemented. Maybe you can open a PR with this change and maybe others you might have found?

Thanks!

[catchergeese]

Hi Ricardo 👋

Thank you for a really prompt reply. I'm happy to hear you're open to improvements in this area.

I'll try to find some time to look into this issue more deeply, analyse what options we have and draft a PR.

See you in Pull requests tab 🤞

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.