Use refinements for ActiveSupport in Rails 6?

rubyonrails-core
1

Refinements have been in Ruby for quite a while now, and are no longer experimental. It seems that it is time for ActiveSupport, the classic example of a use for refinements, to actually begin using them. The disadvantages I can see:

  • using ActiveSupport::CoreExt

    ``

    at the top of each file – not a big deal IMO

  • confusing bugs as a result of forgetting to do so – could be resolved with an Object#method_missing monkey-patch (“did you forget to include the ActiveSupport refinement”)

  • No support for dynamic method calling (some_object.[send|respond_to?|method] :method_added_by_a_refinement never sees the method) – might be a problem. Use caces for dynamic calling of AS extensions?

  • This will be changed in the future. The big advantage here is the ability to use ActiveSupport in gems without fear of conflicts, but it will also prevent things like monkey patches in gems conflicting with ActiveSupport.

2

I am a bit skeptical.

The theory is that reopeining classes is anarchy, who knows what's tossed where! But I look back at +10 years of Ruby and my observation is that it is not a problem. I personally have had zero conflicts, and as a consultant I can tell you I've see quite a few projects. There may be some debugging horror story in some place, but statistically insignificant in my sample of the world.

The reason could be social, AS and facets, say, are socially accepted to reopen stuff. And people know they cannot just publish a library with weird stuff added to Array. In practice, you don't do that. And if you do, it is clearly justified and documented. And in private projects... you know the risks and know that your private monkey patch is not conflicting because you test it, etc.

For whatever reason, for me it is a theoretical concern that is not backed up by reality.

I like the idea of refinements tough, indeed personally used one recently in Rails core (https://github .com/rails/rails/blob/f263e250fe1656339043cf51a2075b9c97abfcce/activesupport /lib/active_support/evented_file_update_checker.rb#L82-L90). So albeit my perception of this would not motivate me to do this refactor, if you do have the motivation and would like to work on it, I think it could be worth exploring and see how it looks with real code and how it would impact the Rails code base.

2 Likes
3

Perhaps it’s worth it to take a look at this project [1] which attempted more or less the same, make it work with ruby 2.2, then run the Rails tests with it instead of the original AS core extensions and see what you get.

[1] https://github.com/amatsuda/activesupport-refinements

4

Thanks Xavier, that pretty much sums up my thinking as well.

5

Let me give you some context and some initial thoughts.

Context

In general, Rails makes extensive use of class reopening.

For some time loading AS was all or nothing, but some years ago there was an effort to separate monkey patches in units to be able to cherry-pick them. This had a few implications:

  1. Users of stand-alone AS have three levels of granularity to import stuff, from the most specific (I only want #blank?), to entity groups (I want all extensions to String), to all AS core extensions. This organization is explained in the AS core extensions guide.

Thanks to this, merely depending on AS no longer modifies core classes. For starters, loading active_support.rb monkey patches nothing. You can depend on the library and cherry-pick just what you need, which is very lightweight. Of course, if the extension needs another one internally, it must be loaded. But generally speaking, that is kept as strict as possible.

  1. Rails is a key client of AS. Being useful to Rails itself is the main criteria of inclusion of something in the core extensions. And each file in the Rails code base is responsible for cherry-picking exactly what it needs. Well, with some exceptions time proved to be worthwhile, the extensions in

https://github.com/rails/rails/blob/master/activesupport/lib/active_support/rails.rb

are imported by all Rails components other than AS itself for project convenience.

  1. Ruby on Rails applications have the ability to say “load only what Rails itself needs, instead of all core extensions”. You can express that with config.active_support.bare = true, though I think I have never seen it used.

Initial Thoughts

In principle, as a first step I would focus on Active Support stand-alone. And in a way that makes the current contract work transparently. Requiring the files you can require today should work exactly as it does today from the point of view of client code (mod ancestor chains maybe, ancestor chains are not public contract, they can be changed). In particular, the test suite of Rails should pass as is.

This work would be organized in the file system in a structured and predictable way, perhaps similar to the current organization.

Then a second step could be to refactor Rails itself. As I imagine it, this refactor would generally consist of changing every require with a corresponding using, modulus a ton of details probably :). We would need to see how the patch actually looks like.

Don’t know if there is any potential impact on performance, this should be also taken into account.

If you’d like to explore this, you can count on me for support for certain.

Xavier

6

Let me give you some context and some initial thoughts.

Context

In general, Rails makes extensive use of class reopening.

For some time loading AS was all or nothing, but some years ago there was an effort to separate monkey patches in units to be able to cherry-pick them. This had a few implications:

  1. Users of stand-alone AS have three levels of granularity to import stuff, from the most specific (I only want #blank?), to entity groups (I want all extensions to String), to all AS core extensions. This organization is explained in the AS core extensions guide.

Thanks to this, merely depending on AS no longer modifies core classes. For starters, loading active_support.rb monkey patches nothing. You can depend on the library and cherry-pick just what you need, which is very lightweight. Of course, if the extension needs another one internally, it must be loaded. But generally speaking, that is kept as strict as possible.

  1. Rails is a key client of AS. Being useful to Rails itself is the main criteria of inclusion of something in the core extensions. And each file in the Rails code base is responsible for cherry-picking exactly what it needs. Well, with some exceptions time proved to be worthwhile, the extensions in

https://github.com/rails/rails/blob/master/activesupport/lib/active_support/rails.rb

are imported by all Rails components other than AS itself for project convenience.

  1. Ruby on Rails applications have the ability to say “load only what Rails itself needs, instead of all core extensions”. You can express that with config.active_support.bare = true, though I think I have never seen it used.

Initial Thoughts

In principle, as a first step I would focus on Active Support stand-alone. And in a way that makes the current contract work transparently. Requiring the files you can require today should work exactly as it does today from the point of view of client code (mod ancestor chains maybe, ancestor chains are not public contract, they can be changed). In particular, the test suite of Rails should pass as is.

Based on https://github.com/ruby/ruby/blob/v2_1_0/doc/syntax/refinements.rdoc , there does not appear to be a way for a required file to trigger using in the parent file:

titleize_string.rb:

module TitleizeString

refine String do

def titleize

…

end

end

using TitleizeString # only active until end of titleize_string.rb!

use_case.rb:

require ‘titleize_string’

puts “foo”.titleize # => NoMethodError

This also implies a “using ActiveSupport::BagORefinements” at the top of a lot of files in the typical application. :frowning:

On the performance front, IIRC part of the reason for the strong lexical scoping of refinements was performance concerns regarding the original dynamic implementation’s effects on method lookup caching.

—Matt Jones

7

Based on

https://github.com/ruby/ruby/blob/v2_1_0/doc/syntax/refinements.rdoc , there does not appear to be a way for a `require`d file to trigger `using` in the parent file:

Right now, each file in Rails core is responsible for loading the extensions it needs. It has no intention to push that to the caller, but it does because loading extensions has a global effect.

In the alternative approach with refinements, the (vague) idea would be the same, each file still loads the extensions it needs, the difference is precisely that it does not have a global side-effect.

By now we are talking about AS, and as a second iteration, Rails core (and all this is speculative at this point). We have not talked about Rails applications, which would still load all extensions unless the bare flag is true using the compatible behaviour that was outlined before.

8

Wow, I can’t believe I haven’t seen this topic before. A lot of good stuff here.

each file in Rails core is responsible for loading the extensions it needs

That’s cool. Thus, we can replace require "active_support/core_ext/whatever" with using ActiveSupport::CoreExt::Whatever. And we can use autoload here, so, no manual require is needed.

Requiring the files you can require today should work exactly as it does today from the point of view of client code

That’s hardly possible: refinements are activated at compile time; require is executed at run time.

Don’t know if there is any potential impact on performance

The only affect I can imagine is the increase in the internal VM memory usage (since we need to keep track of all the activated refinements for a given file). Could it be noticeable? I doubt so, though we need to do some experiments. Method lookup and execution should be roughly the same.

Let me also add some ideas to the plan @fxn shared above.

For the phase 1 (prepare ActiveSupport itself) we can go with an approach I use in Ruby Next: separate patches (the actual code) from adapters (core_ext or refinements), and generate both core extensions and refinements (see, for example, https://github.com/ruby-next/ruby-next/blob/master/lib/ruby-next/core/array/deconstruct.rb).

Then, we automatically (RuboCop’s autocorrect to the rescue) replace require with using. Now, Rails codebase is free of core exts. That worth a major release :slightly_smiling_face:

The next question is whether we encourage library developers and end users to use refinements or allow them to use core exts? I think, libraries should be environment-friendly and use refinements. While users can decide on their own.

P.S. One benefit of de-globalizing ActiveSupport I see is that non-Rails users wouldn’t be afraid of monkey-patches anymore and could use Rails parts

2 Likes