dry-rb / dry-core

A toolset of small support modules used throughout the @dry-rb & @rom-rb ecosystems
https://dry-rb.org/gems/dry-core/
MIT License
170 stars 33 forks source link

Dry::Core::Memoizable regression in 0.8.0 #70

Closed paul closed 2 years ago

paul commented 2 years ago

Describe the bug

When we upgraded from dry-core 0.7.1 to 0.8.0, some tests in our test suite started failing. I think this commit (d2a8d24956f6af4aafcdbdb55abafa7e336ae9c5) changes what memoized value gets returned when a subclass inherits from a class where the memoization was defined.

To Reproduce

require "dry/core/memoizable"

class Base
  include Dry::Core::Memoizable

  def all
    [:base]
  end

  def page
    all + [:page]
  end

  memoize :all, :page
end

class Child < Base
  def all
    super + [:search]
  end
end

child = Child.new
pp all: child.all, page: child.page

__END__

Dry::Core 0.7.1

{:all=>[:base, :search],
 :page=>[:base, :search, :page]}

Dry::Core 0.8.0

{:all=>[:base, :search],
 :page=>[:base, :page]}
# Missing :search

Expected behavior

In our app, this is chaining ActiveRecord Relation objects, in this test case I've simulated it with an Array.

In dry-core 0.7.1, the #page method on Child uses its own memoized #all method, but in 0.8.0 it seems to have used Base#all instead, so its missing the :search it added.

My environment

flash-gordon commented 2 years ago

Hm, thanks for reporting. I definitely didn't have this case in mind. We also don't have specs for this. I'll submit a fix this weekend.

flash-gordon commented 2 years ago

Released in 0.8.1