Tapioca doesn’t generate the expected types from `rbi/stripe/*`
Describe the bug
👋 As you know, the stripe-ruby gem ships .rbi files since v14; however when generating the RBI files via Tapioca, it doesn’t generate any signature except for params of each class’ def initialize… and, interestingly, the attributes of the Stripe::V2::Amount class?
For example, if you look at the PaymentIntent RBI, each attribute is properly typed, yet it’s not picked up by Tapioca. Any clue? Is it a problem with how the .rbi files are defined, or do you think it’s a bug in Tapioca?
From your README:
and to decrease
Tapiocaloading time we pack the gem with the combined RBI atrbi/stripe.rbi.
~I can’t find this file?~ it’s only on the beta branch
While I’m at it, it’s super slow to re-generate the rbi for the gem… do you think it could be because of https://github.com/Shopify/tapioca/issues/1361 / https://github.com/Shopify/tapioca/issues/1285? I wonder if shipping the docs in the rbi files is useful; they’re already included in the actual gem, so they should be raised up in the editor when hovering anyway, no? I can create a separate issue for that if you think it’s worth it.
To Reproduce
- Install the Stripe gem (v14 or v15 leads to the same issue)
- Run
bin/tapioca gem stripeto generate the RBI for the Stripe gem - Open the generated
sorbet/rbi/gems/[email protected]; signatures are missing except forinitializemethods.
Expected behavior
The full signatures should be generated by Sorbet.
Code snippets
OS
macOS
Language version
Ruby 3.3.7
Library version
stripe-ruby v15
API version
N/A
Additional context
No response
Ahh just saw stripe.rbi is only on the beta branch. ~Let me try v14.1.0-beta.1!~ that didn’t help.
Also just found out about https://github.com/Shopify/tapioca/issues/2248, looks like it may well be a bug with Tapioca…
Hi! Sorry for the friction here. I'm going to work on adding annotations for our manually maintained portions of the SDK so we don't have to rely on Tapioca for generating the rest of the type annotations. I'll also make sure the combined spec comes out in the April release.
nice to hear! isn’t using Tapioca the only way to pull types from your rbi/stripe folder… unless we were to manually copy them? 🤔
Sorbet will still detect them (it should do so even now). Tapioca was necessary to generate types for the manually maintained code.
I think this is possibly not related to this issue, but it seems like nested Stripe Objects are not instantiated correctly, e.g. if you retrieve a Stripe::Account, the account.capabilities will be a simple Stripe::StripeObject instead of Stripe::Account::Capabilities.
So, with this example, the type defined here seems wrong, unless I’m missing something?
You are correct! I'll look into correcting that as well once we have cycles to revisit this.
Hi! As of v16.0.0, we explicitly define all attributes, so Tapioca should correctly combine RBIs, and deserialize inner types, so nested objects should deserialize into the types suggested by annotations. We will also have a fix coming for the missing Stripe::V2::Account!
Thanks Helen, we updated last week and that already improved a lot of our Sorbet type handling.
Here’s a few more Sorbet issues (happy to open separate issues for them if you prefer):
-
Although
Stripe::StripeObjectincludes::Enumerable, doing afindon aStripe::ListObject(which inherits fromStripe::StripeObject) does not narrow the type like it usually does for other enumerables likeT::Array. I think because theeachtype needs to be defined explicitly? See: https://sorbet.org/docs/stdlib-generics#implementing-the-enumerable-interface -
the type for a
Stripe::Eventdata.objectis incorrectly typed to only a simpleT::Hashwhich forces us to useT.cast:Notice how the description says it is a full object as value, which is indeed the case 🤔 Should it be typed to a
Stripe::StripeObjectinstead? Ideally theeventitself could be narrowed down with a generic to aStripe::Event[Stripe::PaymentIntent], for example… Thenevent.data.objectwould automatically be typed to aStripe::PaymentIntent. -
Stripe::Account#external_accountsis typed as a simpleStripe::ListObject, but ideally it would be narrowed to a list ofT.any(Stripe::BankAccount, Stripe::Card), like it’s already the case in external account related operations. I wonder if again a generic would be the right solution here so that you can express what kind of objects a list contains 🤔 MaybeT.any(Stripe::BankAccount, Stripe::Card)could even be exposed as aStripe::Account::ExternalAccounttype alias! -
Stripe::Account.retrievereturnsT.untypedinstead of aStripe::Account -
Stripe::Account.create_login_linkreturnsT.untypedinstead of aStripe::LoginLink -
Error types seem to be incomplete: -
Stripe::StripeError#errorreturnsT.untypedinstead of aStripe::ErrorObject
Stripe::ErrorObject#payment_intentreturnsT.untypedinstead of aT.nilable(Stripe::PaymentIntent)(I assume there are other objects than a payment intent); currently it actually is an instance ofStripe::StripeObject, it doesn’t get serialized to aStripe::PaymentIntent.Stripe::ErrorObject#codereturnsT.untypedinstead of aString
And, I don’t know if you’re aiming for full Sorbet type coverage, but:
• Stripe::Object.construct_from (e.g. Stripe::Event.construct_from(…)) isn’t typed and returns T.untyped. I think it should return T.self instead?
• Similarly, Stripe::Webhook.construct_event isn’t typed. We have a shim for it:
module Stripe::Webhook
sig {
params(payload: String, signature_header: String, endpoint_secret: String, tolerance: T.nilable(Integer))
.returns(Stripe::Event)
}
def self.construct_event(payload, signature_header, endpoint_secret, tolerance: Stripe::Webhook::DEFAULT_TOLERANCE)
end
end
Is this something you’d rather goes into https://github.com/Shopify/rbi-central/blob/main/rbi/annotations/stripe.rbi? In an ideal world, the annotations in rbi-central would not be necessary since you now ship your own type annotations.
Oh, another issue I just noticed… When re-generating the RBI file for 16.1.0-beta1, it seems like the generated RBI removes a bunch of ::Stripe:: class prefixes, which leads to a lot of Sorbet errors for missing classes, directly inside the generated RBI. 🤔 Weirdly, this seems to be a new issue because I currently have a different RBI file for the same version ([email protected]) and bin/tapioca gems --verify tells me everything is up-to-date, too. Still, it generates something else when I run bin/tapioca gem stripe. Might be a Tapioca issue…
I tried the v17 beta and had the same issue.
That last issue with generating the RBI with tapioca is a bug in the rbi gem that tapioca runs into. It's discussed here https://github.com/Shopify/tapioca/issues/2248. I have a proposed fix over here https://github.com/Shopify/rbi/pull/513, but an alternative workaround would for Stripe's exported RBI to always use scoped names rather than nested namespaces.
Revisiting this, v17.0.1 now covers Stripe::V2::Amount and Stripe::V2::DeletedObject.
These are all great points! I'll file ListObject to be fixed, as well as the explicit namespaces -- I'm not sure where those went!
The other points are mainly due to the shortcoming that we only type the generated portions of the code at the moment. All manual code is not yet typed. It's on our radar but not prioritized at the moment, as it would require manually auditing and adding types. Please let me know how much it would benefit your workflow, which can help us signal to prioritize it!
Many of these functions, such as retrieve and the child create_login_link in the generated Account class (but notably not functions in manually maintained classes in Event and Error), they come from manually written inherited functions or dynamically created functions. However, there is a workaround - the StripeClient pattern which uses services. These have much less manually maintained code and thus have much better typing:
create_login_linkanalogAccount.retrieveanalog There are more details available in our v13 migration guide.
It's on our radar but not prioritized at the moment, as it would require manually auditing and adding types. Please let me know how much it would benefit your workflow, which can help us signal to prioritize it!
It’s honestly not a huge deal. We can always use Tapioca shims or pull community annotations from rbi-central.
I think it would be good to mention this in the README to set expectations though? Now that I have more context I totally understand why that’s the case. But, considering Stripe created/maintains Sorbet, and that you ship annotations with the library, as a new consumer of the gem I would be a bit surprised to find that they’re incomplete. I remember when we first started using your library, our expectation was that it would be “state-of-the-art” Sorbet, but then ended up having to come up with many workarounds 🙈
Same goes for the rbi-central annotations, which are very incomplete. I wonder if some types could also be conflicting 🤔 Probably better to clearly explain that only automatically generated types are shipped, and the rest is maintained by the community in rbi-central. Wouldn’t it make sense to have people contribute to the annotations directly here instead, though? 🤔
And yeah, it’s on our TODO to switch to the StripeClient / service-based pattern, great to know there’s better typing, one more reason for us to prioritize this!
Appreciate your work, recent improvements have already made our code less verbose! Always happy to try newer versions on our codebase too, although we’re still in the middle of building our Stripe integration.
Thanks for all the details! I'll provide all this feedback to the team and report back once we make more improvements!
Just came across this issue when upgrading the gem. We are having issues with our type checking after upgrading to 16.0.0, since the rbi file generated by tapioca contains lots of errors. Is there a recommended workaround for having a working rbi after this version?
Oh, another issue I just noticed… When re-generating the RBI file for
16.1.0-beta1, it seems like the generated RBI removes a bunch of::Stripe::class prefixes, which leads to a lot of Sorbet errors for missing classes, directly inside the generated RBI. 🤔 Weirdly, this seems to be a new issue because I currently have a different RBI file for the same version ([email protected]) andbin/tapioca gems --verifytells me everything is up-to-date, too. Still, it generates something else when I runbin/tapioca gem stripe. Might be a Tapioca issue…I tried the v17 beta and had the same issue.
Just following up on this too as 17.2.0-alpha.2 has the same generation issues where there's ::Stripe:: class prefixes missing. I've looked at the threads in Tapioca but it doesn't seem like this is being resolved?
My colleague @bcdanieldickison has a PR still awaiting review which should fix it, pinged Morriar about it earlier this week, but no answer yet. https://github.com/Shopify/rbi/pull/513
@helenye-stripe: over in https://github.com/Shopify/rbi/pull/513#issuecomment-3504619839, the solution I proposed is seen as too complicated, so the recommendation is for Stripe to generate its rbi with flattened namespaces, using fully-qualified type names everywhere. This would sidestep the issue described in https://github.com/Shopify/tapioca/issues/2248. Would this be possible?