soutaro / rbs-inline

Inline RBS type declaration
MIT License
175 stars 6 forks source link

Suggestion: Make `rbs/inline.rb` a no-op file #11

Closed st0012 closed 2 months ago

st0012 commented 2 months ago

đź‘‹ I just want to propose a small user-experience improvement: Instead of asking users to add require: false manually, I think alternatively this gem can:

We do the same in Ruby LSP for the same purpose. Feel free to close this if you think it doesn't fit here 🙂

soutaro commented 2 months ago

Hi @st0012!,

In fact, it is avoid loading type definition of rbs-inline gem when the user type checks their application. https://github.com/ruby/rbs/wiki/Release-Note-3.0#rbs-collection-skips-loading-application-gems

Thank you for sharing what Ruby LSP is doing. It would make sense to do it too.

ParadoxV5 commented 2 months ago

IMO, rbs/inline.rb (as well as ruby-lsp.rb, FWIW) should not be (practically) empty, because when a tinkerer wants to write a mod or build another app upon rbs-inline, they would expect require 'rbs/inline' to do something. require 'rbs/inline/internal' does not sound like a confident entry point.

st0012 commented 2 months ago

In fact, it is avoid loading type definition of rbs-inline gem when the user type checks their application.

Ah I see. I'll close this then đź‘Ť

when a tinkerer wants to write a mod or build another app upon rbs-inline, they would expect require 'rbs/inline' to do something.

Sorry I disagree. A library's main purpose it to serve users, and in this case it's not the tinkerers. So if a design can reduce its target users' overhead (e.g. prevent accidentally loading ruby-lsp and cause increased memory usage), then it should be prioritised over ease-of-extension.

Additionally, if a library hope to be extended, then it should provide a blessed way to do it. For example, to extend IRB by adding commands, you require only irb/command instead of the entire IRB.

ParadoxV5 commented 2 months ago

In fact, it is avoid loading type definition of rbs-inline gem when the user type checks their application. https://github.com/ruby/rbs/wiki/Release-Note-3.0#rbs-collection-skips-loading-application-gems

There’s a practice to differentiate “development apps” (e.g., use Rake to automate, use RBS to test) and “runtime libraries” (e.g., Rake plugin, RBS processor) in Gemfiles/.gemspecs. rbs collection (as well as RBS in general when browsing sig/s) should learn to identify those patterns. I’ll check if this’ve been raised in the RBS repo.

ParadoxV5 commented 2 months ago

I’ll check if this’ve been raised in the RBS repo.

It’s mentioned in the thread https://github.com/ruby/rbs/issues/1148#issuecomment-1426756981 but not formally filed.