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

Add :injector_mixin plugin #221

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions lib/dry/system/plugins.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ def initialize(name, mod, &block)
end

# @api private
def apply_to(system, options)
system.extend(stateful? ? mod.new(options) : mod)
def apply_to(system, **options)
system.extend(stateful? ? mod.new(**options) : mod)
system.instance_eval(&block) if block
system
end
Expand Down Expand Up @@ -90,13 +90,13 @@ def self.loaded_dependencies
# @return [self]
#
# @api public
def use(name, options = {})
def use(name, **options)
return self if enabled_plugins.include?(name)

raise PluginNotFoundError, name unless (plugin = Plugins.registry[name])

plugin.load_dependencies
plugin.apply_to(self, options)
plugin.apply_to(self, **options)

enabled_plugins << name

Expand Down Expand Up @@ -134,6 +134,9 @@ def enabled_plugins

require "dry/system/plugins/zeitwerk"
register(:zeitwerk, Plugins::Zeitwerk)

require_relative "plugins/injector_mixin"
register(:injector_mixin, Plugins::InjectorMixin)
end
end
end
2 changes: 1 addition & 1 deletion lib/dry/system/plugins/env.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class Env < Module
attr_reader :options

# @api private
def initialize(options)
def initialize(**options)
@options = options
end

Expand Down
65 changes: 65 additions & 0 deletions lib/dry/system/plugins/injector_mixin.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
# frozen_string_literal: true

module Dry
module System
module Plugins
# @api private
class InjectorMixin < Module
MODULE_SEPARATOR = "::"

attr_reader :name

def initialize(name: "Deps")
@name = name
end
Comment on lines +12 to +14
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rubocop is saying:

Lint/MissingSuper: Call `super` to initialize state of the parent class.

Do we really think we need to do this here?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a feeling we should always delegate to super. Who knows where this class can end up being used later? If it doesn't properly delegate, this would break expectations.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My question is more about whether it's really necessary when inheriting from Module. We already don't do it across the other instances of Module inheritance in this codebase, and it's not something that's appeared necessary to me when using this technique elsewhere.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@timriley I doubt it plays a role in this particular case. It's not a problem to have super calls to quell rubocop, though

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's beneficial to have a habit of calling super, even if it's a noop. It may happen that you really need to call super, but you don't have this habit and you end up debugging why things don't work just realize you forgot to call super. Been there done that 🙂


def extended(container)
container.after(:configure, &method(:define_mixin))
end

private

def define_mixin(container)
inflector = container.config.inflector

name_parts = name.split(MODULE_SEPARATOR)

if name_parts[0] == ""
name_parts.delete_at(0)
root_module = Object
else
root_module = container_parent_module(container)
end

mixin_parent_mod = define_parent_modules(
root_module,
name_parts,
inflector
)

mixin_parent_mod.const_set(
inflector.camelize(name_parts.last),
container.injector
)
end

def container_parent_module(container)
if container.name
parent_name = container.name.split(MODULE_SEPARATOR)[0..-2].join(MODULE_SEPARATOR)
container.config.inflector.constantize(parent_name)
else
Object
end
end

def define_parent_modules(root_mod, name_parts, inflector)
return root_mod if name_parts.length == 1

name_parts[0..-2].reduce(root_mod) { |parent_mod, mod_name|
parent_mod.const_set(inflector.camelize(mod_name), Module.new)
}
end
end
end
end
end
2 changes: 1 addition & 1 deletion lib/dry/system/plugins/zeitwerk.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def self.dependencies
attr_reader :options

# @api private
def initialize(options)
def initialize(**options)
@options = options
super()
end
Expand Down
83 changes: 83 additions & 0 deletions spec/integration/container/plugins/injector_mixin_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
# frozen_string_literal: true

RSpec.describe "Plugins / Injector mixin" do
describe "default options" do
it "creates a 'Deps' mixin in the container's parent module" do
module Test
class Container < Dry::System::Container
use :injector_mixin
configured!
end
end

component = Object.new
Test::Container.register "component", component

depending_obj = Class.new do
include Test::Deps["component"]
end.new

expect(depending_obj.component).to be component
end
end

describe "name given" do
it "creates a mixin with the given name in the container's parent module" do
module Test
class Container < Dry::System::Container
use :injector_mixin, name: "Inject"
configured!
end
end

component = Object.new
Test::Container.register "component", component

depending_obj = Class.new do
include Test::Inject["component"]
end.new

expect(depending_obj.component).to be component
end
end

describe "nested name given" do
it "creates a mixin with the given name in the container's parent module" do
module Test
class Container < Dry::System::Container
use :injector_mixin, name: "Inject::These::Pls"
configured!
end
end

component = Object.new
Test::Container.register "component", component

depending_obj = Class.new do
include Test::Inject::These::Pls["component"]
end.new

expect(depending_obj.component).to be component
end
end

describe "top-level name given" do
it "creates a mixin with the given name in the top-level module" do
module Test
class Container < Dry::System::Container
use :injector_mixin, name: "::Deps"
configured!
end
end

component = Object.new
Test::Container.register "component", component

depending_obj = Class.new do
include ::Deps["component"]
end.new

expect(depending_obj.component).to be component
end
end
end