rotundasoftware / nunjucksify

Everything you expect from a module named nunjucksify and more.
34 stars 10 forks source link

Resolve recoursive template dependencies #2

Closed w0rm closed 10 years ago

w0rm commented 10 years ago

Hello! I'd like to thank you for this plugin. My usage of nunjucks is rather uncommon, I have created a custom extension that allows to do this:

File 'block.nunj'

<div class="block">{% block items %}default{% endblock %}</div>

File 'test.nunj'

{% bem 'block.nunj' %}{% block items %}override items{% endblock %}{% endbem %}  

File 'test.nunj' will be compiled into <div class="block">override items</div>

It does so by introducing new type of node, that captures raw contents during parse, adds {% extends %} and compiles itself as a new template from captured contents. The problem appears when I want to use such tag inside itself and pass it the same template name:

{% bem 'block.nunj' %}
    {% block items %}
        {% bem 'block.nunj' %}{% block items %}override items{% endblock %}{% endbem %}
    {% endblock %}
{% endbem %}

I need this to be compiled into <div class="block"><div class="block">override items</div></div> but I get an error that browserify could not find module with the name 'block.nunj'.

Contents of nested {% bem 'block.nunj' %} are compiled inside the scope of 'block.nunj', so this block tries to require itself by using overridden env.getTemplate and causes browserify error.

Is there any workaround for this problem?

w0rm commented 10 years ago

Forgot to mention, that if I externalise 'block.nunj' when creating bundle, then it works as expected.

.transform(nunjucksify, {env: nunjucksEnv})
.require('block.nunj')
.bundle()
dgbeck commented 10 years ago

Hi @w0rm, I'd love to help you but this is going over my head. I'm not sure what the best work around would be.

w0rm commented 10 years ago

@dgbeck I'm not sure, but maybe store all precompiled templates in global object and use it in patched getTemplate rather than rely on require(name). What do you think?

w0rm commented 10 years ago

Update: this is not related to my custom extension and can be reproduced with nunjucks alone.

Main file test.nunj

{% extends 'block.nunj' %}
{% block content %}
  {% include 'item.nunj' %}
{% endblock %}

File block.nunj

<div class="block">
  {% block content %}
    default content
  {% endblock %}
</div>

File item.nunj

<div class="item">
  item content
</div>

When this is compiled and test.nunj is required from bundle.js, the following error is displayed in console:

Uncaught Template render error: (c4026eaf77ac0fb96e4c76f7b9b58a61dd4141b0)
Template render error: Template render error: Error: Cannot find module 'item.nunj' 

This happens because block.nunj doesn't directly depend on item.nunj and cannot require('item.nunj') inside itself.

dgbeck commented 10 years ago

Gotcha. Uff.

So there must be no call to evn.getTemplate for item.nunj inside the javascript generated for block.nunj, and therefore no require statement is generated. The javascript for test.nunj is calling the javasvript for block.nunj and then the error is happening because item.nunj has not been required.

Unfortunately this is quite a deep problem. The fix you suggested,

store all precompiled templates in global object and use it in patched getTemplate rather than rely on require(name)

would work I think, but then we are back to relying on a global object to store templates.

I would imagine that there is some way to reference the parent template within the javascript of the child template, so another possibility would be to work up the chain of inheritance until the referenced template is found. Like

        compiledTemplate += '   env.getTemplate = function( name, ec, cb ) {\n';
        compiledTemplate += '       if( typeof ec === "function" ) {\n';
        compiledTemplate += '           cb = ec;\n';
        compiledTemplate += '           ec = false;\n';
        compiledTemplate += '       }\n';

        compiledTemplate += '       var tmpl = recursiveRequire( name, env.myParentTemplate );\n';
        compiledTemplate += '       if( ec ) tmpl.compile();\n';
        compiledTemplate += '       cb( null, tmpl );\n';
        compiledTemplate += '   };';

where recursiveRequire is defined at the end

module.exports.recursiveRequire = recursiveRequire = function( name, myParentTemplate ) {
   try {
     return require( name );
   } catch( err ) {
     return myParentTemplate.recursiveRequire( name );
   }
}

I'm not sure where we could find the reference to the myParentTemplate though. It is probably somewhere in the arguments to the root render function, env or context, but I'm not sure.

Alternatively a more straight forward but less elegant work around might be to "force" a reference to item.nunj from within block.nunj, for example:

<!-- {% include 'item.nunj' %} -->

<div class="block">
  {% block content %}
    default content
  {% endblock %}
</div>

Does that work for your use case?

w0rm commented 10 years ago

@dgbeck thanks for the reply! I'm gonna try your recursiveRequire idea, will research if it is possible to traverse up to parent template. Otherwise I guess my only options would be global object or export all templates with b.require.

Forcing dependency doesn't work in my case, because what I'm trying to accomplish is a collection of independent blocks that would be used in all kinds of compositions (e.g. nested grid structure with any content inside).

dgbeck commented 10 years ago

ok good luck! would be a really cool trick if we can pull it off!

w0rm commented 10 years ago

Unfortunately, I could not implement this with recoursiveRequire.

Global context seemed to work by storing each extracted template global.nunjucksifyCache['nested.nunj'] = require('nested.nunj'), but I am worried about relative requires, e.g. when ./partial.nunj means different templates depending on location of parent template. In this case cache will be overridden and wrong partial may appear somewhere.

So what I'm looking for now is a simple recipe to externalise all browserify modules that have .nunj extension.

dgbeck commented 10 years ago

Unfortunately, I could not implement this with recoursiveRequire.

What was the issue you ran into?

Global context seemed to work by storing each extracted template global.nunjucksifyCache['nested.nunj'] = require('nested.nunj'), but I am worried about relative requires, e.g. when ./partial.nunj means different templates depending on location of parent template. In this case cache will be overridden and wrong partial may appear somewhere.

Yeah, that sounds like a show stopper.

So what I'm looking for now is a simple recipe to externalise all browserify modules that have .nunj extension.

I'm not following.. how would that help?

w0rm commented 10 years ago

Speaking about recoursiveRequire, it didn't work because it can't be solved by traversing in the parent direction. The frame mechanism in nunjucks runtime allows to access only parent frames. But given my example, test.nunj depends on both item.nunj and block.nunj, and we need to resolve dependency of block.nunj on item.nunj that are siblings to each other.

UPD: Hmm.. when I wrote it like this, it seems possible. Maybe I didn't try hard enough. My idea was to store require function as variable in nunjucks frame and if require doesn't work then try to use the ones from parent frames.

Exposing all .nunj modules will allow to call require in any context, and it will be resolved as long as all templates are included in the bundle.

w0rm commented 10 years ago

@dgbeck I have managed to fix this, by storing require in frame, and then using it instead of local require. This PR includes changes from #4

PS I will be offline for the next week.

dgbeck commented 10 years ago

Very, very cool. We will test this and merge it. Thank you for this contribution!

w0rm commented 10 years ago

@go-oleg great, thanks!