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

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

Closed
pat opened this issue Nov 3, 2023 · 4 comments · Fixed by #247
Closed

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

pat opened this issue Nov 3, 2023 · 4 comments · Fixed by #247
Assignees
Labels
Milestone

Comments

@pat
Copy link

pat commented Nov 3, 2023

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.

view/lib/hanami/view.rb

Lines 528 to 543 in 89fa278

def call(format: config.default_format, context: config.default_context, layout: config.layout, **input)
rendering = self.rendering(format: format, context: context)
locals = locals(rendering, input)
output = rendering.template(config.template, rendering.scope(config.scope, locals))
if layout
begin
output = rendering.template(
self.class.layout_path(layout),
rendering.scope(config.scope, layout_locals(locals))
) { output }
rescue TemplateNotFoundError
raise LayoutNotFoundError.new(layout, config.paths)
end
end

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
Copy link
Member

timriley commented Nov 4, 2023

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
Copy link
Author

pat commented Nov 4, 2023

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
Copy link
Member

timriley commented Nov 4, 2023

@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.

@timriley timriley self-assigned this Nov 4, 2023
@timriley timriley added this to the v2.1.0 milestone Nov 4, 2023
@pat
Copy link
Author

pat commented Nov 5, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants