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

Including templates with blocks containing parameters lacking default values panics #85

Open
mvrhov opened this issue May 10, 2018 · 4 comments

Comments

@mvrhov
Copy link

mvrhov commented May 10, 2018

The engine crashes with the following error:
"panic: runtime error: invalid memory address or nil pointer dereference"

Reporducer:

Main template

{{block body()}}
{{include "include.jet"}}
{{end}}

include.jet

{{block inputField(label)}}
{{label}}
{{end}}

the problem seems to be that the label doesn't have default value... If {{block inputField(label)}} is changed to {{block inputField(label="foo")}} then there is no crash

@annismckenzie
Copy link
Member

annismckenzie commented May 12, 2018

While I agree that this looks like a bug I'd also like to point out that files with block definitions shouldn't' be included but rather imported. Is that possible for you? Even if you give the block parameter a default value it'll still be evaluated and executed which is probably not what you want? 🤔

@mvrhov
Copy link
Author

mvrhov commented May 13, 2018

I can do an import (thanks for the hint)that's no problem, but the panic remains and it would be nice if it would be fixed and an error reported instead.

@annismckenzie
Copy link
Member

I agree with you – what would you say should the error message be? I'm asking because this is actually a layer 8 problem in that while you certainly can include a template with blocks they will be evaluated straight away which is almost never what you want – I've had a Jet-powered app with tons of templates in production for years now and I've never come across a use case where this is preferred to importing a template and yielding blocks at the appropriate points. Otherwise one might just as well drop the blocks( )and include the template as is.

We could actually think about failing the parsing of templates when including one that contains blocks but that would entail bumping the major to v3 which I really would like to avoid. But if we go ahead and just disallow including templates with blocks that contain parameters without default values then that's almost the same thing while being vastly more complicated to determine by the parser (I haven't actually looked at that part of the code base yet 😅). Thoughts? /cc @jhsx

@mvrhov
Copy link
Author

mvrhov commented Jul 3, 2018

I agree with you – what would you say should the error message be

"label not defined in this context". The same error or something very similar is already reported on different occasions.

I have a constant struggle with the jet. I'm really really used to the twig php template engine and I find a lot of things that were standard/normal there to be lacking here as e.g. they don't work, or the behavior is different than one would expect.
The next problem I'm probably going to face is how to extract the text to be translated from templates so it can be give to the translators.

@annismckenzie annismckenzie changed the title panic: runtime error: invalid memory address or nil pointer dereference Including templates with blocks containing parameters lacking default values panics Jul 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants