Shopify / tapioca

The swiss army knife of RBI generation
MIT License
720 stars 120 forks source link

ActiveStorage dynamic proxy methods not surfaced #376

Open bmulholland opened 3 years ago

bmulholland commented 3 years ago

I'm actually not 100% sure if this issue belongs here, in sorbet-rails, or somewhere else. Please redirect me to wherever would have responsibility for this!

ActiveStorage has been designed to having you go through the ActiveStorage::Attached::One proxy that provides the dynamic proxy to the associations and factory methods

However, generated RBIs do not include those proxy methods, which means operations like model.file.**open** raise a typechecking error, despite that method both existing and working. A workaround is to use model.file**_blob**.open, but this is not the documented API so I'm nervous using it - I imagine it's more likely to change in future.

I'd expect the RBI for ActiveStorage::Attached::One to include these methods so that typechecking can be used when accessing the files as documented.

bmulholland commented 3 years ago

And indeed that blew up on me because the files are encrypted and, while .file is decrypted, .file_blob is not. So I am unable to type check ruby that accesses those files.

bmulholland commented 3 years ago

Just upgraded to 0.4.25 and tried this out, but the issue is not fixed:

Not enough arguments provided for method Kernel#open (overload.1) on ActiveStorage::Attached::One component of T.nilable(ActiveStorage::Attached::One). Expected: 1..4, got: 0 https://srb.help/7004
    16 |    base_record.file.open do |base_file|
KaanOzkan commented 3 years ago

@bmulholland 0.4.25 was a bugfix release and doesn't include the fix above https://github.com/Shopify/tapioca/releases/tag/v0.4.25.

Feel free to point tapioca to main to temporarily test the new DSL generator.

bmulholland commented 3 years ago

Ah understood, thanks.

bmulholland commented 2 years ago

Okay it's out in 0.5 and I'm still getting the same issue as I last posted. I upgraded and ran tapioca gem - is there another step I'm missing?

Morriar commented 2 years ago

👋 Hey Brendan,

I believe that #416 just moved the problem one step further. While we do generate the proxy methods now, we are still missing is a way to represent the delegate_missing_to to the underlying Attachment: https://github.com/rails/rails/blob/5462fbd5de1900c1b1ce1c9dc11c1a2d8cdcd809/activestorage/lib/active_storage/attached/one.rb#L25 which in turn seems to delegate to its underlying Blob: https://github.com/rails/rails/blob/5462fbd5de1900c1b1ce1c9dc11c1a2d8cdcd809/activestorage/app/models/active_storage/attachment.rb#L24.

I'm not sure we can actually fix this easily right now. You'll have to use T.unsafe for the time being.

I'l reopen the issue so we don't loose track.

bmulholland commented 2 years ago

At least there's some progress! Thanks for re-opening :)

bmulholland commented 2 years ago

FWIW, I tried casting to ActiveStorage::Blob, but even that doesn't work: "Method download does not exist on ActiveStorage::Blob"