web2py / py4web

Other
263 stars 129 forks source link

YATL - Unexpected behavior of nested blocks and inheritance after migrating from Web2py #660

Closed EvilDude closed 2 years ago

EvilDude commented 3 years ago

Hello,

I migrated a hobby project from web2py to py4web. Most of it is working but I am seeing unexpected behavior with my template blocks compared to how it was in web2py.

Below a basic layout that demonstrates the behavior. Essentially I have a block that represents a sidebar, which also contains a block 'sidebarcontent' The sidebar itself has general content and inheritors can add their own by redefining the 'sidebarcontent' block. In my project I have three inheritance layers. With the definition below I expect to see content from file B and C, which is what happened in web2py. But in py4web I get the 'to be overridden' content from file A.

If I remove the 'sidebar' block markers and keep the 'sidebarcontent' block then I get the B/C inheritor content as expected, so this seems to be related to the nesting. Adding [[super]] inside 'sidebarcontent' in file B causes a KeyError on 'sidebarcontent'

The py4web documentation on blocks seems the same as for web2py, though neither mentions nested blocks. I would think this would be a common pattern but maybe this was unintended/undefined behavior in the first place?

I've made an stripped-down py4web application from my project for reproducing the behavior, which I can provide, if that would help.

I'm running the following:

Py4web: 1.20210906.1 on Python 3.7.9 (tags/v3.7.9:13c94747c7, Aug 17 2020, 18:58:18) [MSC v.1900 64 bit (AMD64)]

=========================

-----FileA.html----

[[block sidebar]]

[[block sidebarcontent]]

\

this should be overridden \

[[end]]

\

permanent sidebar content \

[[end]]

-----FileB.html-----

[[extend 'FileA.html']]

[[block sidebarcontent]]

\

Custom B content \

[[end]]

-----FileC.html-----

[[extend 'FileB.html']]

[[block sidebarcontent]]

[[super]]

\

Custom C content \

[[end]]

EvilDude commented 3 years ago

py4web_nested-block_super_in_B_file_stacktrace.txt

jpsteil commented 3 years ago

Do you have variables in your extended templates? In py4web these variables must be 'injected' into extended templates.

-Jim

On Tue, Nov 30, 2021 at 6:46 PM EvilDude @.***> wrote:

py4web_nested-block_super_in_B_file_stacktrace.txt https://github.com/web2py/py4web/files/7629802/py4web_nested-block_super_in_B_file_stacktrace.txt

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/web2py/py4web/issues/660#issuecomment-983171047, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABMKF5VIABEMAL2LQI4G5KTUOVV5LANCNFSM5JDFBSAA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

EvilDude commented 3 years ago

I'm not sure I'm aware of the injection requirement but at least in my reproduction I would say there are no variables. The actual filenames and such don't match the example above but other than that it has the same structure and barebones nature.

I made some screenshots that demonstrate the various behaviors on my end, while showing the file contents, if that helps to clarify.

py4web_nested-block_not_overriden

py4web_nested-block_removed_nesting

py4web_nested-block_super_in_intermediate_inheritor

mdipierro commented 2 years ago

Nested blocks was ever a supported feature.

gi0baro commented 2 years ago

@EvilDude Renoir maintainer here. Nested blocks are supported in Renoir, but the encapsulation logic is different from web2py, as in Renoir a block context is referred to its parent. This is why your original code renders the FileA contents, since the sidebarcontent block exists only in the sidebar context of FileA, and consequentially it's not accessible by FileB or FileC (which is also why your call to super from FileB fails, as sidebarcontent doesn't exist in the scope of FileA).

I'm not really sure why you ended up using such tree in your blocks, as I would just used a single block, eg:

a.html

{{ block sidebarcontent }}
<div> this should be overridden </div>
{{ end }}
<div>permanent sidebar content </div>

b.html

{{ extend 'a.html' }}

{{ block sidebarcontent }}
<div> Custom B content </div>
{{ end }}

c.html

{{ extend 'b.html' }}

{{ block sidebarcontent }}
{{ super }}
<div> Custom C content </div>
{{ end }}

Results:

gi0baro@altair ~/Desktop/renoir-blk-test ❯ noir b.html
<div> Custom B content </div>
<div>permanent sidebar content </div>
gi0baro@altair ~/Desktop/renoir-blk-test ❯ noir c.html
<div> Custom B content </div>
<div> Custom C content </div>
<div>permanent sidebar content </div>
EvilDude commented 2 years ago

Hello Giovanni, thanks for your feedback.

To be clear, I'm just a hobbyist, so if the verdict is that this is working as designed then I don't expect anyone to spend more time on this just to educate me. (Had intended to respond this after Massimo's comment but forgot).

To tackle the point regarding just using one block:

In my mind the way it behaved before was more flexible if one wanted to also be able to completely override the sidebar. (I thought this idea would come across in the original description but I see why it didn't)

If you have it like the following:

<div class='sidebar'>
{{block sidebar}}

{{block one}}
{{end}}

{{block two}}
{{end}}

{{end}}
</div>

Then an inheriting file could override any particular block they want, but could also just replace the whole thing by overriding sidebar.

By having each block need to be side by side like, i.e. without the sidebar block definition:

<div class='sidebar'>

{{block one}}
{{end}}

{{block two}}
{{end}}

</div>

Then if you want to use the same template file for all kinds of sidebar-using-pages then inheritors that want to replace the whole sidebar need to redefine every block as empty, which feels like a kind of boilerplate compared to how it was originally.

Alternatively you could define another copy of the template file that does not have all those sub blocks for pages that need to completely replace it. But having multiple copies would mean that you need to maintain those if anything changes (like the sidebar div definition), which is even less appealing.

So I understand that there is a solution by changing the design, but I wanted to make sure that this was an expected change compared to web2py, since I believe having the original behavior makes for more flexible templates by avoiding some of the mentioned issues.

mdipierro commented 2 years ago

Since Renoir is now the standard and this was not a documented feature of web2py I think we should accept the new behavior as the standard and the correct one.