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

Expose StripeResource on instance #920

Merged
merged 1 commit into from
Jul 6, 2020

Conversation

jlomas-stripe
Copy link
Contributor

r? @stripe/api-libraries
cc @stripe/api-libraries @remi-stripe @paulasjes-stripe

This just surfaces the StripeResource on the instance as well, so you can continue to do const stripe = require('stripe')(apiKeyHere); but still be able to add resources while working with beta things.

@remi-stripe
Copy link
Contributor

re-assigning to @richardm-stripe to both approve but also clarify if this should be done in the codegen part instead of here.

@jlomas-stripe
Copy link
Contributor Author

jlomas-stripe commented Jun 12, 2020

I think it additionally needs a change to one of the preambles in the TS codegen stuff; not certain though.

@richardm-stripe
Copy link
Contributor

richardm-stripe commented Jun 12, 2020

I mean, users will get a type error if they try to use it and we don't add such a type. The problem is, the type of StripeResource is super complicated and difficult to type well -- this is an approximation, I think -- but it uses any a lot, and it only reflects the "public interface" of StripeResource...
ebc1a3a

We could just type it as any and call it a day, too. WDYT @jlomas-stripe

@jlomas-stripe
Copy link
Contributor Author

Hey @richardm-stripe! I think in this case it's just a matter of adding StripeResource to here. since this PR exposes it there; I'm not sure if there's further typing that we can even do wrt custom StripeResource implementations.

@richardm-stripe
Copy link
Contributor

Merging this without a typedef for now. (I don't think there's much advantage to adding StripeResource: any over omitting the typedef altogether)

@richardm-stripe richardm-stripe merged commit c6a4071 into master Jul 6, 2020
@richardm-stripe richardm-stripe deleted the jlomas/surface-resources-on-instance branch July 6, 2020 15:53
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

Successfully merging this pull request may close these issues.

4 participants