Skip to content
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

Open
wants to merge 1 commit into
base: 2.1.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/hanami/view/scope_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

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

resolve_scope_class(name: name, rendering: rendering)
end
end
Expand All @@ -40,7 +40,7 @@ def scope_class(name = nil, rendering:)
def resolve_scope_class(name:, rendering:)
name = rendering.inflector.camelize(name.to_s)

namespace = rendering.config.scope_namespace
namespace = rendering.config.scope_namespace || Object

# Give autoloaders a chance to act
begin
Expand Down
26 changes: 26 additions & 0 deletions spec/integration/scope_builder_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,31 @@

expect(scope).to be_an_instance_of scope_class
end

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.

context 'when multiple scopes are rendered in the same view' do

let(:builder) { described_class.new }

it 'allows to build scopes with different classes' do
FirstScopeClass = Class.new(Hanami::View::Scope)
SecondScopeClass = Class.new(Hanami::View::Scope)

view = Class.new(Hanami::View) do
config.paths = SPEC_ROOT.join("__ignore__")
config.template = "__ignore__"

config.scope_class = FirstScopeClass
end.new

scope = view.rendering.scope({})
expect(scope).to be_an_instance_of FirstScopeClass

scope = view.rendering.scope('SecondScopeClass', {})
expect(scope).to be_an_instance_of SecondScopeClass

scope = view.rendering.scope('FirstScopeClass', {})
expect(scope).to be_an_instance_of FirstScopeClass
end
end
end
end