dry-rb / dry-rails

The official dry-rb railtie
https://dry-rb.org/gems/dry-rails
MIT License
271 stars 27 forks source link

Dependency is not registered when namespace is non-standard named #20

Open deepj opened 4 years ago

deepj commented 4 years ago

Describe the bug

I have a namespace which looks like (example) CloudNATS. This is a namespace of Rails application declared in config/application.rb as well.

When I tried to import a dependecy, I got

Dry::Container::Error (Nothing registered with the key :http)

To Reproduce

# app/operations/create_user
class CreateUser
  include CloudNATS::Import[:http]
end
# lib/cloudnats/http.rb
module CloudNATS
  class HTTP
  end
end

Expected behavior

The dependency is successfully imported without any exception. My namespace would

Your environment

solnic commented 4 years ago

This is expected since it's not a regular word that includes an acronym that you need to register in your inflector. I'm not sure if registering just the acronym or the entire word will work, you need to verify that yourself.

deepj commented 4 years ago

The acronym is declared in config/initializers/inflections.rb 🤧

solnic commented 4 years ago

@deepj yeah but the path to files does not match the constant name, lib/cloudnats/http.rb should be lib/cloud_nats/http.rb. Whatever you do, you need to make sure that the inflector correctly converts the dir names into constant names.

deepj commented 4 years ago

@solnic The problem is, at least in my understanding the situation, the Rails' Inflector returns cloudnats for CloudNATS

Loading development environment (Rails 6.0.2.2)
irb(main):001:0> 'CloudNATS'.underscore
=> "cloudnats"

I tried lib/cloud_nats/http.rb as well, but without any success :(

NOTE: The naming of the app was chosen a few years ago btw :)

deepj commented 4 years ago

I know why it's happening 🙀

Screenshot 2020-04-21 at 10 52 09

I guess I would update documentation because it's taken from dry-rails documentation 😹

Suggestion, dry-rails may would expose Zeitwerk inflection API. Because after solving the above fix, I'm getting

expected file ~/lib/cloudnats/http.rb to define constant CloudNATS::Http, but didn't (Zeitwerk::NameError). 

And this is dangerous to set in Rails Inflector because it can affect 3rd-party gems which can use HTTP acronym in their names. It's safer to use Zeitwerk inflection rather than Rails one.

solnic commented 4 years ago

Suggestion, dry-rails may would expose Zeitwerk inflection API

yeah I've thought about it. In general I would like to make dry-rails zeitwerk-only eventually, because it's a much better solution than whatever old Rails is doing.

solnic commented 4 years ago

And this is dangerous to set in Rails Inflector because it can affect 3rd-party gems which can use HTTP acronym in their names. It's safer to use Zeitwerk inflection rather than Rails one.

I should add that you can (and in this case should) set up a custom inflector and configure it to your needs. It's a matter of doing config.inflector = your_inflector