stripe-ruby icon indicating copy to clipboard operation
stripe-ruby copied to clipboard

Tapioca doesn’t generate the expected types from `rbi/stripe/*`

Open Tom-Bonnike opened this issue 9 months ago • 19 comments

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 Tapioca loading time we pack the gem with the combined RBI at rbi/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

  1. Install the Stripe gem (v14 or v15 leads to the same issue)
  2. Run bin/tapioca gem stripe to generate the RBI for the Stripe gem
  3. Open the generated sorbet/rbi/gems/[email protected]; signatures are missing except for initialize methods.

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

Tom-Bonnike avatar Apr 10 '25 13:04 Tom-Bonnike

Ahh just saw stripe.rbi is only on the beta branch. ~Let me try v14.1.0-beta.1!~ that didn’t help.

Tom-Bonnike avatar Apr 10 '25 13:04 Tom-Bonnike

Also just found out about https://github.com/Shopify/tapioca/issues/2248, looks like it may well be a bug with Tapioca…

Tom-Bonnike avatar Apr 10 '25 13:04 Tom-Bonnike

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.

helenye-stripe avatar Apr 22 '25 17:04 helenye-stripe

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? 🤔

Tom-Bonnike avatar Apr 23 '25 09:04 Tom-Bonnike

Sorbet will still detect them (it should do so even now). Tapioca was necessary to generate types for the manually maintained code.

helenye-stripe avatar May 01 '25 19:05 helenye-stripe

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?

Tom-Bonnike avatar May 14 '25 16:05 Tom-Bonnike

You are correct! I'll look into correcting that as well once we have cycles to revisit this.

helenye-stripe avatar May 19 '25 14:05 helenye-stripe

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!

helenye-stripe avatar Oct 13 '25 17:10 helenye-stripe

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):

  1. Although Stripe::StripeObject includes ::Enumerable, doing a find on a Stripe::ListObject (which inherits from Stripe::StripeObject) does not narrow the type like it usually does for other enumerables like T::Array. I think because the each type needs to be defined explicitly? See: https://sorbet.org/docs/stdlib-generics#implementing-the-enumerable-interface

  2. the type for a Stripe::Event data.object is incorrectly typed to only a simple T::Hash which forces us to use T.cast: Image Notice how the description says it is a full object as value, which is indeed the case 🤔 Should it be typed to a Stripe::StripeObject instead? Ideally the event itself could be narrowed down with a generic to a Stripe::Event[Stripe::PaymentIntent], for example… Then event.data.object would automatically be typed to a Stripe::PaymentIntent.

  3. Stripe::Account#external_accounts is typed as a simple Stripe::ListObject, but ideally it would be narrowed to a list of T.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 🤔 Maybe T.any(Stripe::BankAccount, Stripe::Card) could even be exposed as a Stripe::Account::ExternalAccount type alias!

  4. Stripe::Account.retrieve returns T.untyped instead of a Stripe::Account

  5. Stripe::Account.create_login_link returns T.untyped instead of a Stripe::LoginLink

  6. Error types seem to be incomplete: - Stripe::StripeError#error returns T.untyped instead of a Stripe::ErrorObject

  • Stripe::ErrorObject#payment_intent returns T.untyped instead of a T.nilable(Stripe::PaymentIntent) (I assume there are other objects than a payment intent); currently it actually is an instance of Stripe::StripeObject, it doesn’t get serialized to a Stripe::PaymentIntent.
  • Stripe::ErrorObject#code returns T.untyped instead of a String

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.

Tom-Bonnike avatar Oct 14 '25 08:10 Tom-Bonnike

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.

Tom-Bonnike avatar Oct 14 '25 08:10 Tom-Bonnike

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.

bcdanieldickison avatar Oct 14 '25 16:10 bcdanieldickison

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:

helenye-stripe avatar Oct 16 '25 22:10 helenye-stripe

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.

Tom-Bonnike avatar Oct 17 '25 07:10 Tom-Bonnike

Thanks for all the details! I'll provide all this feedback to the team and report back once we make more improvements!

helenye-stripe avatar Oct 23 '25 14:10 helenye-stripe

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?

agramichael avatar Oct 23 '25 15:10 agramichael

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.

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?

yannickgloster avatar Nov 07 '25 09:11 yannickgloster

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

Tom-Bonnike avatar Nov 07 '25 09:11 Tom-Bonnike

@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?

bcdanieldickison avatar Nov 07 '25 20:11 bcdanieldickison