Skip to content
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

Typescript experience is not great #146

Open
maximelebastard opened this issue Jan 9, 2023 · 4 comments
Open

Typescript experience is not great #146

maximelebastard opened this issue Jan 9, 2023 · 4 comments

Comments

@maximelebastard
Copy link

maximelebastard commented Jan 9, 2023

Hi

I guess you guys have put a lot of efforts in adding typescript support for a lib that wasn't originally designed for it - and thank you for that.

But it seems the typescript support is incomplete and not conveinient - which in the end gives more difficulties than help. I think it could be improved by changing a few things I listed there and that I (and probably others) have found confusing.

Expected

As the library provides Typescript definitions, I'd like to rely on them to speed up the developments and learn the API faster through autocompletion - so that I get more time for my core business.

Actual

Through a very simple use case (listing clients), I experienced 3 issues with the provided typings:

Issue 1: Importing the index is not enough

Expectation:

As 99% of the libs on npm, I expect to import the lib name and use it

import GC from "gocardless-nodejs"

const client = new GC(...);
// or
const client = new GC.Client(...);

// and for other exports from the lib
const res = await client.customers.list({ sort_field: GC.constants.CustomerSortField.CreatedAt });

Actual:

import GC from "gocardless-nodejs"
import { GoCardlessClient } from "gocardless-nodejs/client";
import { CustomerSortField } from "gocardless-nodejs/constants";

const client = new GC(...); // does not work
const client = new GC.GoCardlessClient(...); // does not work
const client = new GC.Client(...); // does not work

const client = new GoCardlessClient(); // works from a second import

// and a third import for the consts
const res = await gc.customers.list({ sort_field: CustomerSortField.CreatedAt });

Issue 2: The typescript structure do not match the NodeJS doc of the lib

Expectation:

The README.md of the lib gives the following Getting Started

// Initialise the client.
const client = gocardless(
  process.env.GC_ACCESS_TOKEN,
  constants.Environments.Sandbox,
  { raiseOnIdempotencyConflict: true },
);

Therefore, I expect to use the example - with the power of typescript

Actual:

This does not work

// Initialise the client.
const client = gocardless(
  process.env.GC_ACCESS_TOKEN,
  constants.Environments.Sandbox,
  { raiseOnIdempotencyConflict: true },
);

I had to dig into the lib and to find the issue #125 to find out that Typescript has a totally different contract

import { Environments } from "gocardless-nodejs/constants";
import { GoCardlessClient } from "gocardless-nodejs/client";
const client = new GoCardlessClient("your_access_token_here", Environments.Sandbox);

Issue 3: I get unexpected typing behaviors with documentation

Expectation:

I could pass an optional "after" parameter to gc.customers.list in order to paginate

const clients = await gc.customers.list({ limit: "10", sort_field: CustomerSortField.CreatedAt, sort_direction: CustomerSortDirection.Desc, ...(lastCursor != null ? { after: lastCursor } : {})  });

Actual:

This conditional spread causes a type error causing an implicit any of the return of the function.

@matthieuprat
Copy link
Contributor

Hey Maxime, thank you for the feedback. All very sensible points. We're unlikely to make those changes in the immediate future but we'll definitely consider them for a future iteration of the library.

@Flowerinno
Copy link

Client wanted to integrate GC, API calls return 403 all the time though request schema and valid read-write access token is provided (even in sandbox). About this package... It's just not working.

@jaycoolslm
Copy link

Tbh it's more about documentation - at least make us TS users aware of these caveats. I've been countering these exact issues at the moment with not a clue in my mind what's going on

@matthieuprat
Copy link
Contributor

Hey there, I'm no longer working at GoCardless but @prolific117 or his team might be able to improve the README if they don't have the bandwidth to improve the client itself.

@prolific117, I think the suggestion would be to add a TypeScript section to the README showing how to instantiate the client and how to use constants:

import { GoCardlessClient } from "gocardless-nodejs/client";
import { CustomerSortField, Environments } from "gocardless-nodejs/constants";

const client = new GoCardlessClient({
  process.env.GC_ACCESS_TOKEN,
  Environments.Sandbox,
  { raiseOnIdempotencyConflict: true }
});
const res = await client.customers.list({ sort_field: CustomerSortField.CreatedAt });

(Haven't checked that this example works!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants