Fix rendering of partials in previews

[liram11]

This PR adds a setting to the Lookbook config which allows disabling automatic prefixing of partials on the ActionView layer.

Motivation / Background

Rendering partials in Lookbook 2.2.2 using ActionView short-hand syntax render @users fails with the following error:

Missing partial lookbook/users/_user with {:locale=>[:en], :formats=>[:html], :variants=>[], :handlers=>[:raw, :erb, :html, :builder, :ruby, :jbuilder]}.

Searched in:
  * "/Users/liram/work/lookbook-partials-test/partials/test/components/previews"
  * "/Users/liram/work/lookbook-partials-test/partials/app/views"
  * "/Users/liram/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/lookbook-2.2.2/app/views"
  * "/Users/liram/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/railties-7.1.3.2/lib/rails/templates"
  * "/Users/liram/work/lookbook-partials-test/partials/app/views"
  * "/Users/liram/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/view_component-3.11.0/app/views"
  * "/Users/liram/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/lookbook-2.2.2/app/views"
  * "/Users/liram/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/turbo-rails-2.0.5/app/views"
  * "/Users/liram/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/actiontext-7.1.3.2/app/views"
  * "/Users/liram/.asdf/installs/ruby/3.2.2/lib/ruby/gems/3.2.0/gems/actionmailbox-7.1.3.2/app/views"

In the error above ActionView automatically adds a lookbook prefix to the partial name which makes no sense in the context of lookbook.

A similar issue was described in #560.

Details

This issue can be fixed by modifying the ActionView::Base.prefix_partial_path_with_controller_namespace ActionView setting during the rendering.

I noticed the same case with ActionView::Base.annotate_rendered_view_with_filenames in ActionViewAnnotationsHandler and decided to extract both cases into ActionViewConfigHandler class.

Additional information

The new option is disabled by default so it wouldn't affect the default lookbook behavior.

Also, I was thinking of adding some integrational tests based on the dummy app. But I decided not to do that for now because It may require adding a db layer to the dummy app (to test render @users case). Let me know if it can be valuable and I'll do some experiments in a separate PR.

[netlify]

✅ Deploy Preview for lookbook-docs ready!

Name	Link
🔨 Latest commit	8319d04
🔍 Latest deploy log	https://app.netlify.com/sites/lookbook-docs/deploys/661ff0b788fa430008c1a540
😎 Deploy Preview	https://deploy-preview-606--lookbook-docs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[allmarkedup]

Hey @liram11 this is fantastic, thank you so much for your work on this! And sorry I've taken a while to get to this.

I'm going to check it out in more detail now but at first glance it looks great. To be honest the only thing I'm unsure about is whether this should actually be enabled by default as I think the current behaviour is really a bug. Whilst in theory this is a potentially breaking change I'm not sure I can think of any real world example where people would be relying on the current behaviour - would you agree?

I'm tempted to default to enabling this by default whilst giving people the option to disable it if there is some reason they may need to.

Also, I was thinking of adding some integrational tests based on the dummy app. But I decided not to do that for now because It may require adding a db layer to the dummy app (to test render @users case). Let me know if it can be valuable and I'll do some experiments in a separate PR.

Honestly, I'm pretty embarrassed about the state of the tests and test setup at the moment. Any additions or improvements would be most welcome but you might find it a bit of an uphill struggle!

[allmarkedup]

@liram11 It looks like the tests are failing for Rails 6.0 (which doesn't have the ActionView::Base#annotate_rendered_view_with_filenames method) - do you think you could take a look at that? The code looks fine to me so I think it may just be to do with the way it's being tested.

[liram11]

Hey @allmarkedup,

Thanks a lot for taking a look at that!

I'm tempted to default to enabling this by default whilst giving people the option to disable it if there is some reason they may need to.

I have the same feeling about that. To be honest, I was thinking about making this a default behavior but decided to get some initial feedback before introducing it. Since we agree on that - let me change it to be a default behavior.

It looks like the tests are failing for Rails 6.0

Sure, I'll take a look. I should have some time today.

[liram11]

Honestly, I'm pretty embarrassed about the state of the tests and test setup at the moment. Any additions or improvements would be most welcome but you might find it a bit of an uphill struggle!

Understood, I'll do some experiments with that later and will share the results in case anything sensible comes out😉

[allmarkedup]

I have the same feeling about that. To be honest, I was thinking about making this a default behavior but decided to get some initial feedback before introducing it. Since we agree on that - let me change it to be a default behavior.

Fantastic - in future versions we may be able to remove the config option altogether but it's good to have that for now just in case.

Sure, I'll take a look. I should have some time today.

Thank you! Much appreciated 🙂

[liram11]

Hey @allmarkedup,

I changed the default behavior of the new option - now it is enabled by default.

I also restructured the tests a bit. Now they should work fine even if some ActionView parameters are not available.

[allmarkedup]

@liram11 Fantastic, thank you - this is looking great now. And I appreciate you taking the time to update the config documentation to include this new option too.

I've just given it a spin and I can't see any issues so I'm going to merge this in and get it out in the next release. Thanks again for your contribution!

[allmarkedup]

@liram11 I've just released v2.3.0 that includes this change, much appreciated!

[liram11]

Nice! I'll upgrade it in my project soon. Thank you, Mark!