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

[proposal] Want dry-struct compiler #1210

Closed
YukiJikumaru opened this issue Oct 4, 2022 · 10 comments
Closed

[proposal] Want dry-struct compiler #1210

YukiJikumaru opened this issue Oct 4, 2022 · 10 comments

Comments

@YukiJikumaru
Copy link

I'm using dry-rb which defining typed struct classes, authored by @solnic.

class User < Dry::Struct
  attribute :name, Types::String.optional
  attribute :age, Types::Coercible::Integer
end

class Location < Dry::Struct::Value
  attribute :lat, Types::Float
  attribute :lng, Types::Float
end

Nowadays we can use T::Struct to define typed struct classes, but my project have massive dry-rb structs and T::Struct is not fully compatible with dry-rb.

Therefore I need rbi files for dry-structs, and I wrote a dsl compiler lib/tapioca/dsl/compilers/dry_struct.rb .

YukiJikumaru/tapioca

I wan to know 2 points.
A new dsl compiler is wellcome to tapioca? And you have any contribution guidelines?

@paracycle
Copy link
Member

Thanks for writing the compiler for dry-structs, it looks great!

We only include DSL compilers for a few core gems that we use at Shopify inside Tapioca at this time. Our plan is to extract them, when appropriate, to the gems that they are modelling at some point in the future.

For that reason, we are not accepting DSL compiler contributions, since we won't be able to support them.

However, Tapioca is smart enough to load any Ruby file under the lib/tapioca/dsl/compilers/ folder of any gem, so you can package that up as a gem, if you'd like. Moreover, if you think the dry-rb project would accept it as a contribution, you could send it upstream as well.

Finally, you are correct, we should make this more clear in our contribution guidelines.

@iMacTia
Copy link
Contributor

iMacTia commented Oct 8, 2022

@YukiJikumaru I like the idea of adding the compiler to dry-rb, this way we won't need to add extra gems to our Gemfile just for compilers. Do you think it's worth opening a ticket with dry-rb and asking them if they'd be open to it?

@solnic
Copy link

solnic commented Oct 9, 2022

Thanks for working on this @YukiJikumaru. I would say having tapioca-#{gem_name} would be better than distributing its compilers across different gems. At least it makes more sense in my head. WDYT?

@YukiJikumaru
Copy link
Author

YukiJikumaru commented Oct 10, 2022

@iMacTia @solnic

I like the idea of adding the compiler to dry-rb, this way we won't need to add extra gems to our Gemfile just for compilers.
Do you think it's worth opening a ticket with dry-rb and asking them if they'd be open to it?

I would say having tapioca-#{gem_name} would be better than distributing its compilers across different gems

YES! I want YukiJikumaru/dry_tapioca_compiler YukiJikumaru/tapioca-compilers-dry_struct to be inluded into dry-rb.
How can I help you to integrate the compiler into dry-rb?

@solnic
Copy link

solnic commented Oct 10, 2022

Sorry, what I meant is that it feels like tapioca compilers extend tapioca's functionality, so in case of dry-struct, you'd have something like tapioca-dry-struct but I'm not sure what tapioca developers think about it 🙂 This thread mentions that existing compilers will be eventually extracted to the gems that they support, which means it'd be ie activerecord gem including a tapioca compiler. I don't think that adding tapioca code to individual gems is a good idea though because it'd increase maintenance burden. It's just extra code as part of the main code base and it affects CI, release process and so on. That's why I'd prefer if we had tapioca-{gem_name} convention for gems that provide specialized tapioca compilers. tapioca-activerecord would ship compiler for activerecord, tapioca-dry-struct would ship your compiler for dry-struct and so on.

WDYT?

@iMacTia
Copy link
Contributor

iMacTia commented Oct 10, 2022

That is very fair @solnic and as an OSS maintainer myself I can see your angle. I'd probably need to do something similar for Faraday as well as we do use quite a lot of metaprogramming 😅.
OTOH, I worked enough with typescript in the past to completely hate having to add all the @types/my-library to my projects, and I'd really love to spare this pain to Ruby engineers.

Bundling tapioca compilers inside gems will avoid the situation where Gemfiles gets full of tapioca-* gems that developers need to add manually. Instead, RBI support would become a plug-and-play thing.
The other advantage is that the compiler will likely need updating when the gem is updated, or it might break on some changes. Having it part of the gem itself would allow spotting these issues upstream with the right unit tests.

I do see the issue of the maintenance burden though, and I don't really have a good solution for that in mind.
It would be interesting to know if @paracycle and the rest of the team have already thought about this kind of integration and the potential problem of where the liability/maintainance burden falls wrt the compilers.

Finally, a super-wild thought: an alternative solution would be to have separate gems with some sort of convention (e.g. tapioca-* as suggested above), but have some sort of support in bundler that automatically pulls these extensions if you're working with tapioca/RBIs. Or maybe, tapioca dsl could search, download and run these compilers?

@paracycle
Copy link
Member

@solnic I understand your approach and respect it. However, I think I can make an argument as to why, to me, at least, it seems appropriate that Tapioca compilers should be a part of the respective gem they model.

It is indeed the case that Tapioca DSL compilers extend the functionality of Tapioca. However, they do that to represent the DSL added methods in an application by the DSL patterns from the gem in question. So, in that sense, they model the behaviour of the gem more than the behaviour of Tapioca or anything else. The DSL compiler code is just a glorified way to encode that behaviour in a manner that Tapioca can understand and generate the correct artefacts for.

This is not that different from a gem exporting a Rails or a Hanami generator to make their functionality available to application authors. The generator code would indeed be extending Rails/Hanami, but the functionality would belong to the gem. I view this as being very similar. Another analogous case is gems exporting Rake tasks, which are also extending Rake functionality, but are happily used by many gem authors.

Finally, (as @iMacTia is also saying above), we've seen instances where an upstream gem changed how they model the DSL generation, and thus break some of the compilers bundled with Tapioca. If the compiler was a part of the gem, and tested along with it, it would not have broken as such and that is the best developer experience possible.

Before I sign off, I would like to just add my talk from RubyKaigi 2021 where I was talking about how what Tapioca is doing leads to better analysis and understanding of our applications, that I think is relevant: https://www.youtube.com/watch?v=Ms7_PrryvMM If you have 30 mins to watch, I think my points above could make more sense.

In summary, I respect your opinion as a gem author that you might not want to get that (development, btw, never runtime) dependency on Tapioca for your gems. It is perfectly reasonable to package them as tapioca-dry-struct or something, indeed. Having said that, I still think there is a good case to be made for taking on that dependency.

@YukiJikumaru
Copy link
Author

@solnic

Thank you! After reconsidering, I thought tapioca-compilers-dry_struct should be better.

As recommendations for naming gems by https://guides.rubygems.org/name-your-gem/ .

GEM NAME REQUIRE STATEMENT MAIN CLASS OR MODULE
ruby_parser require 'ruby_parser' RubyParser
rdoc-data require 'rdoc/data' RDoc::Data
net-http-persistent require 'net/http/persistent' Net::HTTP::Persistent
net-http-digest_auth require 'net/http/digest_auth' Net::HTTP::DigestAuth
tapioca-compilers-dry_struct require 'tapioca/compilers/dry_struct' Tapioca::Compilers::DryStruct
tapioca-compilers-virtus require 'tapioca/compilers/virtus' Tapioca::Compilers::Virtus

@YukiJikumaru
Copy link
Author

There is still a lot of discussion and there may be major changes, I have released the alpha version.

https://rubygems.org/gems/tapioca-compilers-dry_struct

@solnic
Copy link

solnic commented Oct 22, 2022

Yeah I can see that now. Thanks for sharing your thoughts and insights. I'll talk to the rest of the team about this after we're done with Hanami 2.0 release. Supporting Sorbet OOTB would be really nice. I just want to be sure that if we decide to do it as part of the gem codebases, then it will actually be maintained 😅

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

No branches or pull requests

4 participants