-
-
Notifications
You must be signed in to change notification settings - Fork 80
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
Fix: Add support for rendering multiple scopes by passing string as names #253
base: 2.1.x
Are you sure you want to change the base?
Conversation
I ran into this too. Without this patch, use of multiple scopes is essentially useless which defeats the power they provide. It would be good to get this folded in and a patch released shortly after. |
@@ -31,7 +31,7 @@ def scope_class(name = nil, rendering:) | |||
elsif name.is_a?(Class) | |||
name | |||
else | |||
View.cache.fetch_or_store(:scope_class, rendering.config) do | |||
View.cache.fetch_or_store("scope_class: #{name}", rendering.config) do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 If it helps, in my local patch, I only use name
which is all you need to store in the cache. Example:
View.cache.fetch_or_store name, rendering.config do
# Implementation details.
end
@@ -27,5 +27,31 @@ | |||
|
|||
expect(scope).to be_an_instance_of scope_class | |||
end | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 If it helps, in my local patch, I needed a way to test that the cache works as expected and ended up with the following spec:
Spec Snippet
RSpec.describe Milestoner::Views::ScopeBuilder do
subject(:builder) { described_class }
let :view do
Class.new Hanami::View do
config.paths = Bundler.root.join "lib/milestoner/templates"
config.template = "n/a"
end
end
let(:scope_a) { Class.new Hanami::View::Scope }
let(:scope_b) { Class.new Hanami::View::Scope }
before do
stub_const "TestScopeA", scope_a
stub_const "TestScopeB", scope_b
Hanami::View.cache.cache.clear
end
describe ".call" do
it "caches scope without duplicates" do
builder.call TestScopeA, locals: {}, rendering: view.new.rendering
builder.call TestScopeB, locals: {}, rendering: view.new.rendering
builder.call "TestScopeA", locals: {}, rendering: view.new.rendering
builder.call "TestScopeB", locals: {}, rendering: view.new.rendering
builder.call scope_a, locals: {}, rendering: view.new.rendering
builder.call scope_b, locals: {}, rendering: view.new.rendering
expect(Hanami::View.cache.cache.values).to eq([TestScopeA, TestScopeB])
end
end
end
Might be a good idea to add a spec for #call
because that ensures the cache is used and doesn't duplicate itself. This also means you'd need to clear the cache between specs which why I put the in the before
block.
Background
Resolves: #252
Bug 1.
When scope name is passed as string (either scope class name, or the path to the file), the name resolving goes through the
cache
, with a hardcoded keyscope_class
, not allowing mutliple scopes to be stored and fetched.Bug 2.
If namespace is not defined (nil),
const_defined?
is called onnil
, causing Undefined method error to be risen.Design
nil
namespace, falling back to the Object, so we can useconst_get
andconst_defined?
methods.