Feature requests: Typescript definitions

[ronzeidman]

Hi,
Since typescript popularity is on the rise and there are no community definitions available for stripe-node I would very much like Typescript definitions. This will help both Typescript users and normal JS users with IDEs that support them like WebStorm and VSCode (even Sublime and Atom have JS hinting plugins that support them).
I find using type definitions much easier than going through the documentation. In addition not all the documentation is up to date since I see the functions can optionally return promises if a callback is not passed and it's not documented.

[bryanerayner]

There is an existing stripe definition in DefinitelyTyped, which is creating conflicts.

Type definitions could be added to this library directly to avoid issues.

[ronzeidman]

It's a bad type definition. It's outdated, lacking and named wrong (since it's a definition for the UI it should have been called "stripe.js" and not "stripe" which is the server side module. Also for the client side I've made a small repo: https://github.com/ronzeidman/stripe-client-ts the server side definitions are more important and longer so I think it would benefit everyone if stripe were maintaining them themselves. Also there is an es6 refactor request issue that refactoring in typescript can solve that would be even better than just including and maintaining definitions.

[bryanerayner]

I think the best solution (which would avoid the @types issue completely) would be to include the .d.ts files in this repo. They would then be included via the "types" setting in the package.json.

How much interest would there be in supporting TypeScript?

[ronzeidman]

At least 4 according to the current thumbs up 😄
Anyone interested please thumb up this issue!
I suggest a twitter survey for stripe users.

[rachaeldawn]

I got this working (intellisense in VScode also works from this)

npm install typings --global
typings init
typings install dt~stripe-node --save --global

That's all you need to do to get it working. :)

[RP-3]

@jschmold Could you share a little more about how you got that working? Specifically, how exactly do you import stripe to get intellisense in VScode to play nicely with it?

[rachaeldawn]

@RP-3

Follow all the instructions I have above, and then pretend it came with the type defs. It works automatically because it looks into the global type definitions for stripe, then intellisense just uses them

[RP-3]

@jschmold Sorry, I'm probably being really thick here. What exactly do you mean "pretend it came with type defs"?. I've followed your instructions above and I've tried the following to import stripe:

import * as Stripe from 'stripe'; //fails with "Could not find a declaration file for module 'stripe'"
import Stripe = require('stripe'); //fails with "Could not find a declaration file for module 'stripe'"
import * as Stripe from 'stripe-node'; //fails with "Cannot find module 'stripe-node'"
import Stripe = require('stripe-node'); //fails with "Cannot find module 'stripe-node'"
const Stripe = require('stripe'); //works but without intellisense

I've also tried the above four combinations after running npm install @types/stripe-node. The errors are the same except that:

import Stripe = require('stripe-node'); //fails with "File '.../node_modules/@types/stripe-node/index.d.ts' is not a module."
import * as Stripe from 'stripe-node'; //also fails with "File '.../node_modules/@types/stripe-node/index.d.ts' is not a module."

It seems to me that there are two things going on here. I have a vanilla node module called stripe sans definitions, and a set of typings called stripe-node. What kind of import statement can I use to import the node module stripe and have it use the definitions from stripe-node ?

[rachaeldawn]

@RP-3
That's really weird. That exact code works for me. Email me so we can fix it, then post the results here? I'm off of work in 4 hours, and I'll do what I can to help.

[email protected]

[RP-3]

@jschmold Thanks for all the help. I've just got something to work... it might be that we're using different versions of tsc ? For what its worth, I'm on 2.3.2.

For anyone else stuck with my dilemma, the following worked:
npm i --save @types/stripe-node

Then import using:

import 'stripe-node'; // make stripe-node types globally visible in the current file
const Stripe: StripeNode.StripeExport = require('stripe'); // manually assign those types to the lib

Reading the section titled (Import a module for side-effects only)[https://www.typescriptlang.org/docs/handbook/modules.html] I guessed @types/stripe-node is written to be imported as a global module and won't work otherwise. Assuming that's right, it makes sense to me that I wouldn't be able to import both the lib and the differently-named definitions in a single statement.

UPDATE:
This also comes with a horrible hack. import 'stripe-node'; will compile to require('stripe-node'), which will then fail at runtime. One god-awful way to fix this for me was to add the following two lines to my package.json:

"postinstall": "npm run compile",
"compile": "tsc && cp -r node_modules/@types/stripe-node node_modules/stripe-node/ && echo \"module.exports = {}\" >> node_modules/stripe-node/index.js",

Frankly, I hate this solution so if anyone knows of anything better please comment and let me know.

[bryanerayner]

@RP-3 , if you got it to work that's great.
I hate this too, but you have just made it possible to actually use this thing.

[bryanerayner]

Any word from a maintainer of this repo as to whether or not a .d.ts file could be included directly in this module?

[rachaeldawn]

I found out what made it work with minimal effort for me.

Before you start, if you don't have it already
npm install typings --global

To get stripe's intellisense working
npm init
npm install stripe --save
typings install dt~stripe-node --save --global
touch tsconfig.json

The TSConfig

{
    "compileOnSave": true,
    "compilerOptions": {
        "allowJs": true
    }
}

The moment the TSConfig exists, my intellisense works with no effort, even if I am using a js file.

intellisense working

[manu-st]

I followed @jschmold and got it working too. Thanks!

[rachaeldawn]

@manu-silicon
You're welcome :) I use intellisense to learn APIs and such quickly, so not having it makes me feel blind. Combining with Typescript also helps, so I wanted to find a solution

[rbluethl]

Any updates on whether or not you're planning to add type definitions?
Doing so would be greatly appreciated.

@jschmold Nice workaround you've found there!
However, to me it doesn't seem like a solution to use a hack,
since every notable library out there provides type definitions already.

Thanks!

[ricklove]

Following @RP-3 initial suggestion worked well.

I just created a file called stripe.ts:

import 'stripe-node';
const Stripe: StripeNode.StripeExport = require('stripe');
export { Stripe };

Then I can import like normal:

import { Stripe } from './stripe';

I do this with most external dependencies so I can hack the import and fix typings when needed.

At least this will work until an official solution is found.

Update:

Nevermind, this blows up when I try to use webpack (and fails at runtime as @RP-3 said)...

Looks like I will use it while coding and then comment out the type information for runtime (making the Stripe any).

[sampsonjoliver]

For anyone struggling with stripe and using the types, within TypeScript (or ES6+), what has worked for me across both Flow and TypeScript is

Install the types

npm install --dev @types/stripe-node
or
yarn add @types/stripe-node -D

Import the package and use the exported constructor

import * as Stripe from 'stripe';
const stripe = new Stripe(stripeKey);

// Then do whatever you like:
const customer: Promise<StripeNode.customers.ICustomer> = stripe.customers.create( ... )

All the types exist under the StripeNode module namespace, because the @types/stripe-node index.d.ts has a top-level declare namespace StripeNode { ... }. So when you want to specify a type on a function parameter or a variable, then you will use StripeNode.Foo.

The @types/stripe-node index.d.ts also declares a module called stripe with a non-default export of an interface that contains a constructor. So when you import * as Stripe from 'stripe', you'll get that interface, with the constructor. Invoking the constructor will return a stripe instance with all the methods you expect to see typings and autocomplete for.

[iamclaytonray]

What is the current status on this issue? It has nearly been a year since @ronzeidman opened it and the issue is relatively easy to fix. Is this not going to have a resolution? If not, let the community know and close the issue. It seems like maintainers have just dropped back and not given a clear direction on what's going on concerning this.

[karthikiyengar]

The current typings on DefinitelyTyped are pretty old and unfortunately, I am unable to augment them easily.
Cannot augment module 'stripe' because it resolves to a non-module entity.

We need better types for such a critical piece of software :-)

[sampsonjoliver]

@karthikiyengar if you are using the stripe-node types, extending them is easy.

// myCustomStripeNodeTypings.d.ts

declare namespace StripeNode {
  interface Stripe {
    // add additional top-level classes here
  }

  // Define additional namespaced types or classes here
}

And of course, feel free to make PRs into the DefinitelyTyped/stripe-node repository with enhancements and additional features.

[moberemk]

Could just open a PR that copies the DT typings into this library; are those still very out of date?

[ricklove]

The original problem was that the types were completely wrong. They were for the client library of stripe and not the node library. However, they used the npm name for the node library so there was no way to replace them. (Typescript requires the npm package name match the @types/package name). So we had to hack a way around it. If this has been resolved, then there is no problem. I haven't used stripe in a while so I can't answer if the types have been corrected.

…

On Fri, Feb 23, 2018 at 3:27 PM Mark Oberemk ***@***.***> wrote: Could just open a PR that copies the DT typings into this library; are those still very out of date? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#296 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAq9iOHjtu_p6Jf4LfJsBAjZAfqU3MrDks5tXy1VgaJpZM4LqXL1> .

[iamclaytonray]

@ricklove - It hasn't been resolved. The types are still completely wrong. I decided to just move everything concerning stripe into JS, which sucks, but I guess I'll just deal 🤷‍♀️

[ahaverty]

@sampsonjoliver comment works, but I'm getting a typeError on my parameter types when running with mocha (although implementing stripe types works, and no warnings in VSCode)
It looks like a new @types/stripe was added to DefinitelyTyped repo a few days ago, deprecating stripe-node types.
Anybody have documentation? I've just removed type checking for now 🤔

[ChrisTalman]

@moberemk Pulling the definition from DefinitelyTyped sounds good to me. At the very least, it's a starting point. It'd have the benefits of centralising issue reporting and contributions. Once it's here, people can report issues, and we can work through contributing to the definition to resolve these.

[pradt]

@sampsonjoliver your method worked for me. Thanks

[evdama]

What's the current status on this issue? Our codebase is exploding in size and we've come to the conclusion that, same as for the non-stripe portions, we can't scale anymore without static typing in place... to many regressions and unnecessary bugs make it into code etc.

[sampsonjoliver]

@markusgattol npm install / yarn add @types/stripe is the solution. The @types/stripe-node typings have been moved to @types/stripe which resolves a lot of the confusion around this issue as devs should no longer accidentally be pulling in the wrong type library.

I have mixed feelings on pulling those types into this package. As they are not "first-party", it may be a more sufficient solution to update the readme of this project with a section on using Typescript, and to refer to the @types/stripe package for typings, thus endorsing them as "third-party".

[rattrayalex-stripe]

Great news! We're finally going to be working on this soon. Really wish we could have done this much earlier.

We're planning to include a .d.ts file in this package, probably modeled after DefinitelyTyped ones (but with comprehensive coverage of resources, endpoints, params, and fields, of course).

Are there any shortcomings to the architecture used by the Stripe DefinitelyTyped definitions that we should avoid carrying over?

[MatthiasKunnen]

What would be immensely helpful is for jsdoc on the types to reflect the documentation at https://stripe.com/docs/api.

[rattrayalex-stripe]

After looking at this a little more, we're leaning towards using types instead of interfaces (and thus without the "I" prefix) and including our model types and param types at the top level:

const customerParams: Stripe.CustomerCreateParams = {
  description: 'test',
};
const customer: Stripe.Customer = await stripe.customers.create(customerParams);

// Namespaced resources:
const checkoutParams: Stripe.Checkout.SessionCreateParams = {
  // ...
};
const session: Stripe.Checkout.Session = await stripe.checkout.sessions.create(checkoutParams);

We're currently thinking that model and param types will not share code (for the most part); even if customer.description is the same in the model, create params, and update params, it will be declared inline in all three places (with a few exceptions for widely-shared concepts like Address and Metadata). Every param and field will have a docstring (assuming we document it in the apiref)!

Would love thoughts/feedback from the community!

[ChrisTalman]

I'm curious - what's the reasoning for favouring type aliases over interfaces in this case?

Personally, I tend to only use type aliases for union types and helper types. The syntax around interfaces and extending them just strikes me as being more formal, elegant, and meaningful than their equivalents with type aliases:

interface Thing extends BaseThing
{
	a: number;
};

type Thing = BaseThing &
{
	a: number;
};

Of course, at the end of the day, there do not seem to be many differences between them in functionality, and they can be happily used with one another.

[rattrayalex-stripe]

Good question, @ChrisTalman !

The reason is fairly simple: we don't intend to make use of type abstraction for our model and param types.

The DefinitelyTyped version, being handwritten, strives to avoid duplication with abstractions like this:

interface IObject
interface IResourceObject extends IObject

interface IDataOptions
interface IDataOptionsWithMetadata extends IDataOptions

interface IAccountShared
interface IAccount extends IResourceObject, IAccountShared

interface IAccountUpdateOptions extends IDataOptionsWithMetadata, IAccountShared
interface IAccountCreationOptions extends IAccountUpdateOptions

That way, they don't need to manually add things like id and object to every response and include, expand, and metadata to every request. They also don't need to re-write params that are shared between Create and Update, or fields that are shared between a response and a POST.

However, since we are using code generation for our definitions, it's actually more complex to abstract these things out, and simpler to inline them everywhere. So instead of the above, we'd just have this:

type Account = {
  id: string, 
  object: 'account',
  business_type?: 'individual' | 'company';
  country: string;
  // ...
}
type AccountCreateParams = {
  business_type?: 'individual' | 'company';
  country?: string;
  // ...
}
type AccountUpdateParams = {
  business_type?: 'individual' | 'company';
  // ...
}

My hope is that this will be a bit easier to navigate – you'll see all the params for AccountCreate in one place, for example, instead of having to poke across 3-4 layers of abstraction to see them all. On the other hand, I'm a bit concerned that having everything duplicated could make it just too massive to read/navigate. I hope to have a beta ready soon so folks can provide feedback.

Less salient, but also conceptually, Stripe.Company and Stripe.CompanyCreateParams aren't really interfaces; they should generally be considered more "final" types that are you aren't expected to extend, for the most part.

Thoughts?

[MatthiasKunnen]

@rattrayalex-stripe, If I understand correctly, create, update and result types will not share a common interface.

This is a good design choice. The initial idea for sharing a common interface, DRY, does not hold because some properties will be part of some interfaces but not others and sometimes they will all have the same shared property but the description (jsdoc) is different. It easily becomes a mess. Not sharing a common base like you described will be much easier to maintain. It will also be easier to sync with api docs.

[MatthiasKunnen]

Recommendation: order of properties. You will have to choose how to order the properties of the interfaces. I'm personally a fan of alphabetically sorted properties. Another option is to sort them exactly like the API docs. Which order you choose is up to you of course but I suggest to pick one and be consistent. It will make reading the types much easier.

If possible I'd recommend configuring a linter to enforce the order.

[sampsonjoliver]

I'm also in favour of using types. But, I'd argue the choice depends on how the underlying code is being written -- if you're writing polymorphic javascript using ES classes and extending off of these, then you should use interfaces because that's the kind of relationship that these constructs are made to express.

But if you're writing prototypal structures, or in other words, object-first javascript, then a type is going to express the composition of these a lot more meaningfully.

For example, if you just take

const a = { ... }
const b = { ... }
const c = Object.apply({}, a, b)

then it follows quite naturally that the type of c is more equivalent to

type C = A & B;

than

interface C extends A & B { 
  /* empty body */ 
}

[ChrisTalman]

@sampsonjoliver Just to note, the interface equivalent of & is ,:

interface C extends A, B {}

[MatthiasKunnen]

Another recommendation. Export all types and don't have complex type on a property. E.g.

Don't use this

export interface Foo {
    prop: {
        a: string;
        b: number; 
    };
}

Do use this

export interface Bar { // Make sure Bar is exported, no private types should be used
    a: string;
    b: number;
}

export interface Foo {
    prop: Bar;
}

[rattrayalex-stripe]

Thanks for the discussion!
We're going back and forth internally on interfaces vs types.
One thing I'm curious about; what is the advantage of pulling out nested hashes?

In practice, we're choosing between this:

interface Customer {
  object: 'customer',
  shipping: {
    address: {
      city: string | null,
      // etc
    },
    carrier: string | null
  }
}

and this:

interface Customer {
  object: 'customer',
  shipping: Customer.Shipping,
}
namespace Customer {
  interface Shipping {
    address: Shipping.Address,
    carrier: string | null
  }
  namespace Shipping {
    interface Address {
      city: string | null,
      // etc
    }
  }
}

(Note that all types from a .d.ts are exported, so there will be no private types).

The latter lets you easily reference the type Stripe.Customer.Shipping.Address in your own code, but may be harder to read/navigate.

What are the concrete pros & cons of one vs the other?

[MatthiasKunnen]

The advantage of the second approach is that it makes it possible to assemble a request in parts. Example:

const buildShippingObject: Customer.Shipping = {
    address: {
        city: 'Barcelona',
    },
    carrier: 'Royal Mail TM',
};

const customer: Customer = {
    object: 'customer';
    shipping: buildShippingObject,
};

If Customer.Shipping were not exposed on its own, it would not be possible to type buildShippingObject. This would result in no intellisense/feedback for that object.

The only disadvantage to the second approach is that you might need to ctrl click to get to the specific definition. IMO the benefits of type safety far outweighs this minor disadvantage.

[ChrisTalman]

It may differ for each person, but for me, I actually tend to find the latter more readable than the former. This is because everything is neatly afforded its own space. In the former format, there is less of a dividing line, and everything is more mashed together.

Besides readability, though, the access pattern is very desirable as well, as you note.

[MatthiasKunnen]

I actually tend to find the latter more readable than the former.

Agreed

[MatthiasKunnen]

Another reason to prefer the second approach is to have methods that return a subset of a customer for example.

getCustomerShipping(customerId: string): Customer.Shipping {
    // ...
}

[rattrayalex-stripe]

Okay, I have a beta ready to share, and would love feedback from anyone willing to try it out! You've all been super helpful so far and would love to make this great for everyone.

Send me an email at [email protected] to get started!

[MatthiasKunnen]

Will this be a closed beta and not something like stripe@beta on npm?

[rattrayalex-stripe]

Yes, it's a closed beta, mostly so we know who's trying it and can ask questions (what do you think of X, etc).

[rattrayalex-stripe]

Now a public beta! See instructions to give it a try at #736

[rattrayalex-stripe]

TypeScript support has been released in version 8.0.0! Docs here.

Thanks so much to the community for the patience, and to beta-testers for the fantastic feedback!

Huge thanks also to everyone who's contributed to the DT types; really impressive work.

[ark120202]

Import Stripe as a default import (not * as Stripe, unlike the DefinitelyTyped version)

I just want to note that TypeScript team recommends to avoid doing that for CommonJS modules, because it requires JS users to use it as require("stripe").default (see sindresorhus/memoize#31 for more details)

[rattrayalex-stripe]

Thanks @ark120202 – yeah, I spoke with members of the TS team about this. They encouraged requiring esModuleInterop: true, but in beta-testing we found that broke a lot of unrelated things for users.

In our case, we support both import Stripe from 'stripe' and const Stripe = require('stripe') (as well as import {Stripe} from and const {Stripe} =) by doing this:

module.exports = Stripe;
module.exports.default = Stripe;
module.exports.Stripe = Stripe;

which should be handled well by ~all consumers.

(In the future, better to open a new issue for things like this).