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

OAuth authorizeUrl request: site != library documentation discrepancy #921

Open
kaznovac opened this issue Jun 15, 2020 · 3 comments
Open
Assignees
Labels
type definition Marks issue related to outdated or incorrect TypeScript type definition

Comments

@kaznovac
Copy link
Contributor

in the api doc state parameter defaults to read_only
https://stripe.com/docs/connect/oauth-reference#get-authorize-request

and in this library state defaults to read_write

if (!params.scope) {
params.scope = 'read_write';
}

I can easily create pull request for this code, but maybe the site documentation needs to be fixed.

p.s.
@remi-stripe I couldn't find documented way to contribute to stripe type definitions (I see they have been generated from openapi, but I'm not sure if openapi is generated too). Can you point me how to contribute to types (I'd like to add some string literal types, document setClientId on Stripe (knowing this exists would save me a great workaround in code), etc...)

@remi-stripe
Copy link
Contributor

@kaznovac Thanks for the report! We can't really change this behaviour in the library since it'd be a breaking change for any developer that relies on the default behaviour in stripe-node.

I don't think the docs or the libraries need to be change in this case and it's fine if they don't match exactly though I agree it could be confusing. Anyone using the library and this feature would be testing the flow to make sure it does what they want.

As for the definitions, this is not something you can easily change yourself unfortunately. The definitions are generated from the openapi spec which is itself automatically generated from our own internal code.

On the other hand, the OAuth flow and APIs are not in the openapi spec as they are a separate flow. This means those definitions are not automatically generated and instead fixed by hand. We'd happily take a PR for type definitions improvements if you're up for it!

@remi-stripe remi-stripe self-assigned this Jun 15, 2020
@kaznovac
Copy link
Contributor Author

@remi-stripe Thanks for the answer.

I agree with your stance against introducing breaking change.

Here is a list of remarks for, as I understand, generated types:

  • accounts.retrieveCapability capability parameter can use string literal type
  • SessionCreateParams line_items deprecated fields are not marked as deprecated
  • Charge status can have string literal union type
  • in Transfers %s as 'slang' for cents (got confused with this thought some replacement went wrong; back in C days this is placeholder for a string xD)
  • ... a few more I'd need to hunt in my code

For the OAuth types, I'll look into my use-cases and post a PR.

cheers

@remi-stripe
Copy link
Contributor

Thanks for the detailed report! We're aware of a few of those but some are tricky to fix. We don't really flag deprecated parameters or properties well today unfortunately for example.

I'll flag this internally though!

@kamil-stripe kamil-stripe added the type definition Marks issue related to outdated or incorrect TypeScript type definition label Apr 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type definition Marks issue related to outdated or incorrect TypeScript type definition
Projects
None yet
Development

No branches or pull requests

3 participants