hanami / view

Views, templates and presenters for Ruby web applications
http://hanamirb.org
MIT License
173 stars 80 forks source link

Missing templates result in (incorrect) missing layout error #246

Closed pat closed 1 year ago

pat commented 1 year ago

Hey folks, just giving the migration to 2.1.0.rc1 a shot, and have come across a misleading error - something that cropped up when I last poked around with the earlier beta releases too:

If a template file doesn't exist, the error that I get is that the layout can't be found.

https://github.com/hanami/view/blob/89fa27842d96074ef9d912c99caf6e822229f190/lib/hanami/view.rb#L528-L543

If I comment out the rescue and the line below it - lines 540 & 541 - then I get the original TemplateNotFoundError bubbling up instead, and that is actually useful because it tells me the missing template. I presume that rescue's in there for a reason though, so I'm not sure what the appropriate fix should be!

(FWIW: Ruby 3.2.2, hanami-view 2.1.0.rc1, macOS Ventura)

timriley commented 1 year ago

Hi @pat, thanks for reporting this!

So I looked into this, and it seemed like TemplateNotFoundError was appropriately being raised for missing templates. I added a new test case to ensure this, too.

However, in looking into this, I did realise that LayoutNotFoundError would incorrectly be raised if there was a render for a missing partial inside the layout file.

Was this possibly the scenario you encountered? If you could share a little more detail here, it would help make sure we fix this properly.

In any case, to fix the issue I did notice, I just pushed #247, which removes LayoutNotFoundError, which I think provides only marginal utility. Now without the begin/rescue around the layout rendering, we can be confident that TemplateNotFoundError will be appropriately raised in all cases.

Would you be able to try that branch in your app see if that sorts it for you?

pat commented 1 year ago

Thanks @timriley, I can confirm it was due to missing partials - or rather, partials in a shared subdirectory that needed me to include that folder in the path (perhaps something I could configure better somewhere?)

If I get a moment to pause from the current wrangling (content security policies 🤔), I'll give your branch a try.

timriley commented 1 year ago

@pat Ah yes, regarding your shared subdirectory: for this upcoming release of hanami-view, we've moved on from the dry-view-style lookup strategy for partials (where it looks in "shared/" directories and ascends up the directory tree until it finds a match). This was for two reasons: (1) performance, and (2) ease-of-understanding. Frankly, this was always a very niche feature and it came with a high cognitive cost. Using partials should be easy, so we made it might more straightforward: every render can supply the full path to a template, and that's the one and only place that is looked up.

In your case I'd suggest you update each render call to include the full path of the expected partial file.

pat commented 1 year ago

Thanks Tim, those changes make sense, I'll keep an eye out for other partials as I go.