Refactor memoized helpers #937
Conversation
…re put in the ancestor chain
…will make behaviour more predictable
| MemoizedHelpers.module_for(example_group).__send__(:define_method, memoized_method_name, &block) | ||
|
|
||
| # Apply the memoization. The method has been defined in an ancestor | ||
| # module so we can use `super` here to get the value. |
There was a problem hiding this comment.
The comment about super is inaccurate and a bit confusing now.
|
I was hoping to find a way to solve this by delaying when the I'm going to try to take a shot at that in the next day or two to see if I can find a solution. If I don't get around to it by Wednesday, I'm fine merging this solution, as this definitely solves the issue. |
|
We tried that today @myronmarston but by being after everything else it breaks because things like shared contexts require lets to be before them. E.g. theres a complex relationship with load order, seems that could become brittle and is arguably more complex. Happy to help find a better solution. |
|
@JonRowe -- yep, I just tried it for a while just now, and you're right, there's quite a complex ordering dependency :(. This PR seems like the best solution to me now, too. I'm glad I tried for a bit, though -- it helped me develop a proper appreciation for the difficulty of this problem! |
|
@JonRowe -- can you add a changelog entry for this? |
|
Yep, I intend to I'm also going to update the comments as asked. |
|
(I figured it was easier to merge this then go from there to do so) |
|
Also, I think we'll want this fix in 2.14 and 2.99 as well... If we're going to fix up the comments a bit as I suggested above, it'd probably be good to do that before merging this across into those branches. |
This is a cleanup of #871 that @JonRowe and I worked on.
It fixes #908 and #817 and is references in (rspec/rspec-rails#738)
We're not 100% thrilled with it, but we could not think of a better way to fix the relevant issues while still allowing
superwithinlet.Thoughts?