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

Stop loading non-library RBS files #1148

Closed
soutaro opened this issue Nov 16, 2022 · 2 comments · Fixed by #1222
Closed

Stop loading non-library RBS files #1148

soutaro opened this issue Nov 16, 2022 · 2 comments · Fixed by #1222
Milestone

Comments

@soutaro
Copy link
Member

soutaro commented Nov 16, 2022

Using RBS based tools like rbs_rails and steep requires adding gem entries in rbs_collection.yaml.

gems:
  - name: rbs
    ignore: true
  - name: steep
    ignore: true

This is annoying and causes problems, and it looks wired because the gems section is mainly used to ignore libraries.

I'd like to propose changing the library RBS loading protocol:

  • Skip loading RBS files from gems that is declared in Gemfile with require: false

require: false is usually written for gems that is not loaded from the application, especially in Rails apps, and we can use it as a marker that implies RBS files of the gems are unnecessary.

The steps to load RBS of libraries will be as following:

  1. Require gems that is declared in gems section of rbs_collection.yaml without ignore: true
  2. Require gems and their dependencies from Bundler context that doesn't have require: false (and skip if it's ignored)

With this change, the rbs_rails and steep users would change their Gemfile lines as:

gem 'steep', require: false
gem 'rbs_rails', require: false

It looks better and more intuitive than having ignore: true lines in rbs_collection.yaml. 😄

@soutaro soutaro added this to the RBS 3.0 milestone Nov 17, 2022
@ParadoxV5
Copy link
Contributor

With this change, the rbs_rails and steep users would change their Gemfile lines as:

gem 'steep', require: false
gem 'rbs_rails', require: false

For real – because I require manually the traditional way instead of Bundler.require, I omit writing the require: false part.

Though, how would this change impact the gemspec delegate in the Gemfile?
Additionally, how shall the new format treat add_development_dependency and add_runtime_dependency?


An alternate solution is to refactor rbs_collection.yaml (to the same or a new file) to explicitly list gem RBSs to exclude without declaring ignore: true repeatedly. E.g.: ignore: [rbs, steep]

@ParadoxV5 ParadoxV5 moved this to 🗳️ Submitted in General Projects Feb 10, 2023
@ParadoxV5 ParadoxV5 removed the status in General Projects Feb 10, 2023
@soutaro soutaro linked a pull request Feb 11, 2023 that will close this issue
@soutaro
Copy link
Member Author

soutaro commented Feb 11, 2023

@ParadoxV5 Thank you for your feedback!

I know require: false is optional because some of us, including me, don't use Bundler.require. In this case require: false is a RBS collection specific marker. I feel it's acceptable to abuse that feature for RBS, but if you want, you can continue having ignore: true lines in rbs_collection.yaml.

In this case -- if you have gem "steep" in Gemfile and { name: "steep", ignore: true } in rbs_collection.yaml, RBS won't load RBS files of steep and it's dependencies. (This is different from RBS 2.x.)

However, I'm now thinking if it's really a good design or not...

Though, how would this change impact the gemspec delegate in the Gemfile?

gemspec adds an dependency like gem call without require: false. I don't think this changes anything with that feature, but let me know if you find something unexpected is happening to you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants