-
Notifications
You must be signed in to change notification settings - Fork 756
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
Fix StripeResource.extend type #1245
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I have access to a beta feature, which I wanted to use with the stripe library. In the process, I found that the result of `Stripe.StripeResource.extend()` did not have a constructor, so TypeScript would not let you do this: ``` const Foo = Stripe.StripeResource.extend(); new Foo(stripe) ``` For context, I use this like so: ```ts class Foos extends Stripe.StripeResource.extend({ path: "foos" }) { create = Stripe.StripeResource.method({ method: "POST", path: "" }) as ( params: Stripe.FooBetaFeatureCreateParams, options: Stripe.RequestOptions ) => Promise<Stripe.Response<Stripe.FooBetaFeature>>; update = Stripe.StripeResource.method({ method: "POST", path: "/{foo}" }) as ( id: string, params: Stripe.FooBetaFeatureUpdateParams, options: Stripe.RequestOptions ) => Promise<Stripe.Response<Stripe.FooBetaFeature>>; retrieve = Stripe.StripeResource.method({ method: "GET", path: "/{foo}" }) as ( id: string, options: Stripe.RequestOptions ) => Promise<Stripe.Response<Stripe.FooBetaFeature>>; } class StripeWithFooBeta extends Stripe { foos: Foos; constructor(key: string, opts: Stripe.StripeConfig) { super(key, opts); this.foos = new Foos(this); } } export const stripe = new StripeWithTreasuryBeta(process.env.STRIPE_SECRET!, { apiVersion, }); declare module "stripe" { namespace Stripe { namespace FooBetaFeature {} } } ``` since the transformation of `this.path` which takes place [here](https://github.com/stripe/stripe-node/blob/master/lib/StripeResource.js#L42) does not play nicely with ES6 classes.
Hello Alex! Thanks for the contribution and the detailed description. This looks good -- I added an entry in typescriptTest.ts and am merging. This will be bundled with the next release. |
jnraine-stripe
added a commit
to jnraine-stripe/stripe-node
that referenced
this pull request
Nov 11, 2021
This improves three friction points when working with Typescript: 1. `new StripeResource(stripe)` now works. Prior to this change, attempting to construct `StripeResource` with its one required argument would fail a type check. Support for a constructor was added in stripe#1245 and extended in stripe#1249 (never merged). Thanks @rattrayalex! 2. `StripeResource.extend` now returns a class that is properly typed when instantiated. Prior to this change, you could extend `StripeResource` but your extensions wouldn't appear on the object after construction. Now, it does. 3. `StripeResource.method` returns a function that returns `Response<object>`. Prior to this change, extensions created with `StripeResource.method` would have a return type of `object`. Returning `Response<object>` makes it possible to call functions like `lastResponse` without casting the value. 4. `StripeResource.method` now accepts generic type that sets the response object's type. For example, if you're hitting a customer endpoint, you can do this: `StripeResource.method<Customer>(...)`. When you use that extension, it will return `Response<Customer>`. This eliminates the need to cast when calling StripeResource extension methods.
jnraine-stripe
added a commit
to jnraine-stripe/stripe-node
that referenced
this pull request
Nov 11, 2021
This improves three friction points when working with Typescript: 1. `new StripeResource(stripe)` now works. Prior to this change, attempting to construct `StripeResource` with its one required argument would fail a type check. Support for a constructor was added in stripe#1245 and extended in stripe#1249 (never merged). Thanks @rattrayalex! 2. `StripeResource.extend` now returns a class that is properly typed when instantiated. Prior to this change, you could extend `StripeResource` but your extensions wouldn't appear on the object after construction. Now, it does. 3. `StripeResource.method` returns a function that returns `Response<object>`. Prior to this change, extensions created with `StripeResource.method` would have a return type of `object`. Returning `Response<object>` makes it possible to call functions like `lastResponse` without casting the value. 4. `StripeResource.method` now accepts generic type that sets the response object's type. For example, if you're hitting a customer endpoint, you can do this: `StripeResource.method<Customer>(...)`. When you use that extension, it will return `Response<Customer>`. This eliminates the need to cast when calling StripeResource extension methods. # Conflicts: # types/lib.d.ts # types/test/typescriptTest.ts
richardm-stripe
pushed a commit
that referenced
this pull request
Nov 11, 2021
This improves three friction points when working with Typescript: 1. `new StripeResource(stripe)` now works. Prior to this change, attempting to construct `StripeResource` with its one required argument would fail a type check. Support for a constructor was added in #1245 and extended in #1249 (never merged). Thanks @rattrayalex! 2. `StripeResource.extend` now returns a class that is properly typed when instantiated. Prior to this change, you could extend `StripeResource` but your extensions wouldn't appear on the object after construction. Now, it does. 3. `StripeResource.method` returns a function that returns `Response<object>`. Prior to this change, extensions created with `StripeResource.method` would have a return type of `object`. Returning `Response<object>` makes it possible to call functions like `lastResponse` without casting the value. 4. `StripeResource.method` now accepts generic type that sets the response object's type. For example, if you're hitting a customer endpoint, you can do this: `StripeResource.method<Customer>(...)`. When you use that extension, it will return `Response<Customer>`. This eliminates the need to cast when calling StripeResource extension methods. # Conflicts: # types/lib.d.ts # types/test/typescriptTest.ts
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I have access to a beta feature, which I wanted to use with the stripe library.
In the process, I found that the result of
Stripe.StripeResource.extend()
did not have a constructor, so TypeScript would not let you do this:This fixes that.
For context, I use this like so:
since the transformation of
this.path
which takes place here does not play nicely with ES6 classes.