Remove ActiveSupport dependency?

[cllns]

Here are the ActiveSupport dependencies in FactoryGirl:

In lib/factory_girl/evaluator.rb
1:require 'active_support/core_ext/hash/except'
2:require 'active_support/core_ext/class/attribute'

In lib/factory_girl/factory.rb
1:require 'active_support/core_ext/hash/keys'
2:require 'active_support/inflector'

In lib/factory_girl.rb
2:require 'active_support/core_ext/module/delegation'
3:require 'active_support/deprecation'
4:require 'active_support/notifications'

In spec/support/macros/deprecation.rb
1:require "active_support"

For Rails apps, using ActiveSupport is a no-brainer since it's already required.
For other Ruby apps, ActiveSupport is a pretty big dependency. (Granted, only for testing, but it's still there)

I can work to remove the dependency on ActiveSupport. Is that a worthwhile effort? Are you interested in a PR that does that?

Things like ActiveSupport::Notifications can still be supported, by just checking if they're defined at the top of the file.

(The concrete use-case for this is several of us in the Hanami Gitter chat would like to use FactoryGirl but don't want to add ActiveSupport, since it monkey-patches)

Thanks!

[joshuaclayton]

@cllns thanks for opening this issue.

For a while, we'd avoided including AS because of the same reasons you'd mentioned. We've obviously become more lax on this, and I think you bring up a good point about assessing whether it's a necessary dependency.

Looking through the source for some of those files, I don't think it makes sense right now to introduce similar behavior while avoiding AS - that's a significant amount of code that has the risk of being outdated, buggy, or otherwise needs maintenance over time. The overhead cost of doing that versus leveraging an existing gem is very high.

Were there a community-adopted alternative that didn't rely on reopening classes, it makes sense to consider introducing it given it was actively-maintained and widely-used, across all the versions of Ruby we support.

[cllns]

Thanks for the quick and thoughtful response @joshuaclayton.

Hanami::Utils offers some of the functionality that's needed (like inflectors), but not all of it. Also, it only supports MRI 2.2, MRI 2.3 and JRuby.

[joshuaclayton]

Bummer that some is missing - for the 5.0 release of FG/FGR, I'm planning on dropping support for MRI Ruby below the latest 2.2.x; however, I think we should continue to push towards RBX compatibility, at least for FG 5.0.

Again, thanks for putting forth the investigative work on this!

[cllns]

Now that I'm looking into it, I think all the ActiveSupport features (except pub/sub Notifications /Instrumentation) can be supported with a combination of Hanami::Utils and pure-Ruby.

Note: the 'require' links to where lines where the methods are called.

in lib/factory_girl/evaluator.rb

1. require 'active_support/core_ext/hash/except' is not in Hanami::Utils, but can be a one-liner in pure-Ruby: overrides.tap { |h| h.delete(:strategy) }
2. require 'active_support/core_ext/class/attribute can be replaced by Hanami::Utils::ClassAttribute

In lib/factory_girl/factory.rb

1. require 'active_support/core_ext/hash/keys' can be replaced by Hanami::Utils::Hash#symbolize_keys!
2. require 'active_support/inflector' can be replaced by Hanami::Utils::String.classify and Hanmi::Utils::Class.load!

In lib/factory_girl.rb

1. require 'active_support/core_ext/module/delegation' can be replaced by pure-Ruby's def_delegators in Forwardable
2. require 'active_support/deprecation' can be replaced by Hanami::Utils::Deprecation
3. require 'active_support/notifications' There's no alternative in Hanami::Utils but, if I'm not mistaken, this can be supported if the user has ActiveSupport loaded. If they don't have it, then it just won't work. Not sure if this is OK?

Historically, Hanami::Utils supported Rubinius until recently, in order to focus on MRI and JRuby.

I can reach out to see if they'd want to support Rubinius again, if you're interested in converting ActiveSupport to Hanami::Utils.

[cllns]

Ping @joshuaclayton (figured you didn't see my response since this Issue is closed)

[joshuaclayton]

@cllns thanks again for the work here. I've thought about this and believe it makes sense to continue with ActiveSupport.

The largest is due to developer adoption and use of AS itself. For both developers who are interested in contributing to FG and for those who're interested in understanding what the library does, seeing a library that's not AS would likely be confusing and increase cognitive load ("What is Hanami?", "Why is AS avoided?", "Does Hanami (or another tool) do something different/better than AS that I should further source dive?").

Secondarily, as most developers using this are doing so within the context of a Rails application, introducing another dependency doesn't seem worth it. I understand the size aspects of AS, but it's also a safe, trusted, widely adopted piece of software that is working as expected.

Thanks again for bringing this up for consideration; I truly appreciate it. I'm going to leave the issue closed.

[cllns]

Thanks @joshuaclayton, I completely understand.

Personally, I think adding another dependency for vast majority of users is the more compelling argument. Making people question ActiveSupport and teaching them about the dangers of reopening classes isn't a bad thing, in my opinion :)