Giter VIP home page Giter VIP logo

Comments (41)

j-mori avatar j-mori commented on September 28, 2024 1

Perfect gonna follow the updates cause we are very interest to this framework!

from framework.

cossssmin avatar cossssmin commented on September 28, 2024 1

Yes, of course, the issue still stands - just wanted to have it mentioned. Would love to see a PR, but yeah please wait until I publish the new release. Thanks!

from framework.

cossssmin avatar cossssmin commented on September 28, 2024 1

Fixed it, should work as expected now. Feel free to reopen if you spot any issues ✌

from framework.

cossssmin avatar cossssmin commented on September 28, 2024 1

I tried, it, answered there in #74 . Not only does it not compile that template that extends a template, but I don't want users to have to add workarounds like this, with false and {% extends %}.

Thanks for the PR, but I think I'm just going to make specifying a layout: mandatory. It's not a big deal and it doesn't do any magic stuff, which I think is better.

from framework.

cossssmin avatar cossssmin commented on September 28, 2024 1

Fixed it, will publish soon. @lowv-developer managed to preserve the 'automagic' of layout extending - you should only be required to specify the layout key only if your template is being extended by another template.

from framework.

cossssmin avatar cossssmin commented on September 28, 2024 1

when there is inheritance you need to declare layout in every level.

This. You need to always declare the layout you're extending, if you use inheritance (extending a template from a template). Just tested and it works as expected, so I'll close this as resolved - feel free to reopen if you spot other issues with it. And big thanks to both of you for your help ✌

from framework.

cossssmin avatar cossssmin commented on September 28, 2024

We could do something like:

html = layout ? `{% extends "${layout}" %}\n${frontMatter.body}` : frontMatter.body

So then you'd set layout: false in the template's Front Matter to prevent this. Or as you mentioned, look for {% extends and do it without user intervention.

Perhaps I'm not thinking this through - can you please share a real-life scenario where you'd need a template to extend another template?

from framework.

cossssmin avatar cossssmin commented on September 28, 2024

Actually, can you please provide an example where it's broken, and how you'd expect it to be output?

from framework.

j-mori avatar j-mori commented on September 28, 2024

We could do something like:

html = layout ? `{% extends "${layout}" %}\n${frontMatter.body}` : frontMatter.body

So then you'd set layout: false in the template's Front Matter to prevent this.

Just tested out,
html = layout ? `{% extends "${layout}" %}\n${frontMatter.body}` : frontMatter.body
we ll' do the trick but we should also change toDisk.js:53
let layout = config.layout || config.build.layout into
let layout = config.layout && config.build.layout otherwise we can't set layout= false via Front Matter because false we'll be ignored by the || operator.

Or as you mentioned, look for {% extends and do it without user intervention.

I think, in addition, we should look for {% extends ... %} string in template because in nunjucks it's not possible anyway in just a single template extends multiple templates so its a good check to do.
Probably a regex like {%\s+?extends.+\s+?%} should work but maybe it'll slow down compile time for big layouts?

Perhaps I'm not thinking this through - can you please share a real-life scenario where you'd need a template to extend another template?

The scenario are various, i could have 2 similar newsletter template with just a couple of differences in few blocks, in this case i could extends my newsletter template and redefine just the blocks with desired changes. With nunjucks this is possible at infinite sub-levels so i could extends a template who extends another template and so on.
You could workaround with partials as well but it's just bad to lose a such cool feature who provide huge scalability for a small problem.

from framework.

j-mori avatar j-mori commented on September 28, 2024

Actually, can you please provide an example where it's broken, and how you'd expect it to be output?

of course, take for example your default template transictional.njk, and wrap the Confirm your email button inside a {% block button %}{% endblock %} block.
now create another template transictional_account.njk:

{% extends "src/templates/transactional.njk" %}

{% block button %}Confirm your account{% endblock %}

Now my expectation are to get 2 differents templates:
One with "Confirm your email" button and another with "Confirm your account".

But Instead of the "Confirm your account" template i get a blank template becasue maizzle add the layout extensions on top of my tamplate, nullifing my extends line code.
Obviously this isn't the best feature case scenario becuse the button could be a compnent with a text parameter, but it's just to demostrate how and why the things breaks

from framework.

cossssmin avatar cossssmin commented on September 28, 2024

Thanks for the explanation, it's clear now.

I'm currently working on updating toDisk() to use the toString() method, to consolidate the codebase and make it easier to write tests in the future. I'd like to release this first (ideally this week or the next), and we can then handle this issue 👍

from framework.

cossssmin avatar cossssmin commented on September 28, 2024

Btw you could use Components for your buttons example - just pass in the text. Let me know if you want me to show you an example.

from framework.

j-mori avatar j-mori commented on September 28, 2024

Yeah i know in that particularly case components are probably a better way to reach the result but they was just simple examples, when it comes to more complex templates where i would like to changes an entire code block instead of just a button (for exeample a column in a template with columns), template inheritance is a much better way to handle the case.

Anyway we should definetly fix this just because template inheritance it's a documented and cool feature that Nunjucks offers and at the moment is not working.

If you want i can create a pull request with the changes mentioned above on the current maizzle version or if you prefer i can wait a couple of week for the new relase

from framework.

cossssmin avatar cossssmin commented on September 28, 2024

Merged the PR that refactors toDisk(), can you work off of master or do you need me to publish a release? I'd honestly prefer if we publish a v0.5.0 that includes both that and the inheritance fix.

from framework.

j-mori avatar j-mori commented on September 28, 2024

Thanks!

from framework.

cossssmin avatar cossssmin commented on September 28, 2024

Sorry, had to roll it back in v0.5.1 because of several issues:

  • maizzle serve was getting an undefined layout, so it was not using any
  • all templates were using the same (default) layout, even if they extended a different one

I'm open to suggestions :)

from framework.

cossssmin avatar cossssmin commented on September 28, 2024

One possible solution would be to recursively recompile until there's no layout found.

For example, this works:

html = `{% extends "${config.layout}" %}\n${html}`
html = nunjucks.renderString(html, { page: config, env: options.env, css: compiledCSS })

while (fm(html).attributes.layout) {
  const front = fm(html)
  html = `{% extends "${front.attributes.layout}" %}\n{% block template %}${front.body}{% endblock %}`
  html = nunjucks.renderString(html, { page: config, env: options.env, css: compiledCSS })
}

Biggest downside is we'd need to hardcode the block identifier for inherited Templates. So if the Layout they eventually extend doesn't use that identifier, an empty <body> would be output.

For example, this works:

<!-- src/layouts/default.njk -->
<body>
  {% block template %}{% endblock %}
</body>

<!-- src/templates/first.njk -->
---
layout: src/layouts/default.njk
---

{% block template %}
  Hello, {% block firstname %}Jane{% endblock %}
{% endblock %}


<!-- src/templates/second.njk -->
---
layout: src/templates/first.njk
---

{% block template %}
  Hello, {% block firstname %}John{% endblock %}
{% endblock %}

But if someone prefers to name their block in the Layout differently, say {% block myTemplateContent %}, then second.njk would contain an empty <body>, since we would have {% block template %} hardcoded and that wouldn't exist in the default.njk Layout.

Unfortunately, since you can have as many {% block %} tags as you want, and there's no mandatory order to write them in, we can't get the identifier name either - it can very well be that the first block in a Template is {% block head %} (where you can add stuff to <head>).

It also slows the build process a bit, but if you don't do a lot of template->template extends, it's fine.


I think having to always use {% block template %} in your Layouts feels like a very small thing to ask in exchange for proper template inheritance, so I'd go for it.

Thoughts?

from framework.

lowv-developer avatar lowv-developer commented on September 28, 2024

I cant reproduce your example :/

from framework.

cossssmin avatar cossssmin commented on September 28, 2024

You need the while loop in the toString() method, as I have it above - have you added that locally? I haven't commited anything yet...

from framework.

lowv-developer avatar lowv-developer commented on September 28, 2024

yes i did

i added this (i get error with {% extends "${config.layout}" %}:

const layout = config.layout || config.build.layout

and then below

...const nunjucks = await NunjucksEnvironment.init()

if (typeof options.beforeRender === 'function') {
  await options.beforeRender(nunjucks, config)
}

html = `{% extends "${layout}" %}\n${html}`
html = nunjucks.renderString(html, { page: config, env: options.env, css: compiledCSS })

while (fm(html).attributes.layout) {
  const front = fm(html)
  html = `{% extends "${front.attributes.layout}" %}\n{% block template %}${front.body}{% endblock %}`
  html = nunjucks.renderString(html, { page: config, env: options.env, css: compiledCSS })
}

html = ht..

from framework.

lowv-developer avatar lowv-developer commented on September 28, 2024

my setup is:

<!-- src/layouts/default.njk -->
// default file

<!-- src/templates/trasactional.njk -->
---
layout: src/layouts/default.njk
---
{% block template %}
{% block button %}Confirm your email{% endblock %}
{% endblock %}


<!-- src/templates/transactional_account.njk -->
---
layout: src/templates/transactional.njk
---

{% block template %}
{% block button %}Confirm your account{% endblock %}
{% endblock %}

from framework.

cossssmin avatar cossssmin commented on September 28, 2024

Added 8bdb8dd, make sure you're using v0.5.1 and your toString() looks like in the inheritance branch. You shouldn't get any error with config.layout.

Otherwise files look alright, what command are you using?

from framework.

j-mori avatar j-mori commented on September 28, 2024

Sorry, had to roll it back in v0.5.1 because of several issues:

  • maizzle serve was getting an undefined layout, so it was not using any
  • all templates were using the same (default) layout, even if they extended a different one

I'm open to suggestions :)

hi! Wasn't the problem caused by the bad layout assignment?
just figured out that my advice to use
let layout = config.layout && config.build.layout was just bad.
do you think something like this will solve the issue?

let layout = config.layout != null ? config.layout : config.build.layout

from framework.

lowv-developer avatar lowv-developer commented on September 28, 2024

"@maizzle/framework": "^0.5.1",

maizzle serve

i copy/paste the raw file from "Rework template inheritance"

ex.
In src/templates/transactional_account.njk
I only see the Confirm your account text (no template). Im seeing the all the head stuff in view source

from framework.

cossssmin avatar cossssmin commented on September 28, 2024

hi! Wasn't the problem caused by the bad layout assignment?

Unfortunately no, you could extend as many levels deep as you'd want, so each level will contain a different layout. In order to go up the chain and reach the Layout, while actually rendering block contents, you need to re-render the template, talking the newly-found Front Matter into account.

from framework.

j-mori avatar j-mori commented on September 28, 2024

Added 8bdb8dd, make sure you're using v0.5.1 and your toString() looks like in the inheritance branch. You shouldn't get any error with config.layout.

Otherwise files look alright, what command are you using?

i get an error Error: template not found: undefined, config.layout is undefined

from framework.

lowv-developer avatar lowv-developer commented on September 28, 2024

you need to declare layout in every template!

Added 8bdb8dd, make sure you're using v0.5.1 and your toString() looks like in the inheritance branch. You shouldn't get any error with config.layout.
Otherwise files look alright, what command are you using?

i get an error Error: template not found: undefined, config.layout is undefined

from framework.

cossssmin avatar cossssmin commented on September 28, 2024

i get an error Error: template not found: undefined, config.layout is undefined

Just tested with your exact files, it works perfectly fine. Not sure what's causing it for you.

you need to declare layout in every template!

Right, src/layouts/default.njk is no longer automagically set as the Layout being extended, you need to extend it in your Template.

from framework.

lowv-developer avatar lowv-developer commented on September 28, 2024

"automagically" is always better! :D

from framework.

j-mori avatar j-mori commented on September 28, 2024

Right, src/layouts/default.njk is no longer automagically set as the Layout being extended, you need to extend it in your Template.

The problem was this.

I wasn't setting template for every default template via FM and miazzle was crashing, so is this inteded?

from framework.

j-mori avatar j-mori commented on September 28, 2024

"@maizzle/framework": "^0.5.1",

maizzle serve

i copy/paste the raw file from "Rework template inheritance"

ex.
In src/templates/transactional_account.njk
I only see the Confirm your account text (no template). Im seeing the all the head stuff in view source

Declaring layout in every template make maizzle compile but i get the same result:
transiciotnal.njk work fine but transictional_account.njk show only declared block as plain text + extended template's FM heading as text, layout/default.njk seems not being loaded

from framework.

cossssmin avatar cossssmin commented on September 28, 2024

You're both probably running into a caching issue or something, just tested this and it works with the default layout:

src/templates/first.njk :

---
layout: src/layouts/default.njk
---

{% block template %}
Parent Template
{% block button %}First{% endblock %}
{% endblock %}

src/templates/second.njk :

---
layout: src/templates/first.njk
---

{% block template %}
{% block button %}Second{% endblock %}
{% endblock %}

{{ super() }} added in the template block of second.njk also works.

from framework.

cossssmin avatar cossssmin commented on September 28, 2024

So to get inheritance working properly, the trade-off here is to always have to specify the Layout you're extending from a Template.

I believe I had it work automatically being inspired by the Jigsaw static site generator, which powered the original Maizzle (can't remember if it was a feature, or just something you could just do).

Since we have no better solution, I think we'll go with this - it's a very small trade-off to make for what you get in return.

from framework.

j-mori avatar j-mori commented on September 28, 2024

Hey cosssmin i pulled a potential fix based on 0.5.0, could u give a try?

With that fix templates will still load default layout from config.js but now its possible to set

layout: false

inside templates whose doesn't need to automagically include the default conifg.js layout!
So now its possible to do something like:

---
layout: false
---
{% extends 'src/templates/transactional.njk' %}
{% block button %}My button{% endblock %}

from framework.

cossssmin avatar cossssmin commented on September 28, 2024

There's obviously still an issue with using render() without extending a Layout (simply render a string), working on a solution...

from framework.

cossssmin avatar cossssmin commented on September 28, 2024

Please check v0.5.2, it should have fixed the issues.

from framework.

j-mori avatar j-mori commented on September 28, 2024

Thanks for the fix! just tested out and it seems working pretty fine, but i have another related small problem, maybe i m doing something wrong but seems that the main layout doesn't get compiled.

This is my setup with defaults templates:

src/templates/transactional.njk:
added layout via fm and wrapped command inside a block

---
layout: src/layouts/default.njk
---

{% block cta %} Confirm email address{% endblock %}

src/templates/transactional_account.njk:

---
layout: src/templates/transactional.njk
---
{% block cta %} Confirm your account{% endblock %}

The expected result would be two identical html with differents commands text, but in transactional_account.njk some class miss inside <html>,<body> tags, probably layout didn't get compiled as should?

Results:
-src/templates/transactional.njk:

<body class="bg-gray-200 h-full">

-src/templates/transactional_account.njk:

<body>

from framework.

cossssmin avatar cossssmin commented on September 28, 2024

Will need to test, but I have a hunch this is purgeCSS or email-comb removing classes. Might have to do with the recursive re-rendering.

This is for the maizzle build command right?

from framework.

j-mori avatar j-mori commented on September 28, 2024

No i m having the issue with miazzle serve, did'nt tested build command

from framework.

lowv-developer avatar lowv-developer commented on September 28, 2024

everything seems to be working fine!

<!-- src/templates/transactional_account.njk -->
---
title: trasactional
layout: src/templates/transactional.njk
bodyClass: bg-gray-200 h-full
htmlClass: bg-gray-200 h-full
---


{% block button %}Confirm your account{% endblock %}

from framework.

lowv-developer avatar lowv-developer commented on September 28, 2024

No i m having the issue with miazzle serve, did'nt tested build command

try to delete the folder @maizzle and npm install

when there is inheritance you need to declare layout in every level.

the layout is optional when you have only one level (without inheritance)

from framework.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.