Current API imposes a Unix-filesystem-like convention on relative path interpretation

[boutell]

I know you don't support relative paths in templates by default (at least, not last I looked), but I'm aware that nunjucks is very extensible, to the point of being able to add tags and so on. So I thought you'd be up for hearing about the issues I ran into when I chose to implement this myself.

Template A wants to include template B which lives in another folder, so it uses a relative path, with a custom loader that permits this. All is good so far.

Now template B wants to include template C. It just uses the name of the file, because template C is in the same folder.

Here we have problems (:

A nunjucks loader is only passed the name being included. It is not passed any information about the "parent" path the file is being loaded from.

In the case of Apostrophe, I'm not dealing with relative paths exactly: I'm dealing with "cross-module includes," where instead of "../../whatever.html" I want to include "modulename:whatever.html". So my loader resolves what folder "modulename" corresponds to. But it amounts to the same thing: given the current API, it (almost) isn't possible to support the use of non-absolute paths in template B when it attempts to load template C.

But if the loader received "parentName," the name under which template B was loaded, then it would be trivially possible to do any sort of relative path resolution we wish.

OK, I say "almost isn't possible" because I did come up with a hack. But it is a terrible hack, and you're going to see it, and wince, and wish that loaders received parentName. Here it is:

  // Fix all calls to extend, include and import so that
  // paths without an explicit module name use the specified
  // module name. Uses the official Nunjucks lexer. -Tom

  self.resolveTemplatePathsViaModule = function(src, moduleName) {
    var out = '';
    var lexer = templates.nunjucks.lexer.lex(src);
    var token;
    var state = 'boring';
    while (token = lexer.nextToken()) {
      // Push the token back out again. Unfortunately
      // there is no .raw property, so we have to quote
      // strings and regexes again ourselves (and hope
      // there's nothing else weird out there...)
      if (token.type === 'string') {
        out += '"' + token.value.replace(/\"/g, '\\\"') + '"';
      } else if (token.type === 'regex') {
        out += '/' + token.value.replace(/\//g, '\\/') + '/';
      } else {
        out += token.value;
      }
      if (token.type === 'block-start') {
        state = 'block';
      } else if ((token.type === 'symbol') && (_.contains([ 'include', 'import', 'extends', 'from' ], token.value))) {
        state = 'expr';
        // This is naive and can introduce additional module names
        // if the path here in the template already has a module name.
        // This is worked around explicitly in getSource(). -Tom
        out += ' "' + moduleName + ':" +';
        state = 'boring';
      } else if (token.type === 'block-end') {
        state = 'boring';
      }
    }

    return out;
  };

Wow, isn't that awful? I use your tokenizer to modify the include, import and from statements in the template being included, prepending a module path to them.

This totally works, and could also be used to support relative paths in a filesystem loader that allows multiple directories of templates. But I'm forced to rebuild the source as I go. And because there is no ".rawValue" I have to rebuild quoted strings and regexps... and pray those are the only tokens where ".value" and the original raw code would be different. Yikes. (:

I could ask for .rawValue in the lexer, and maybe I should because that would be useful to someone, but I think supplying parentName to loaders would be a lot more practical.

How do you feel about that idea? Would you accept such a pull request?

Thanks!

[jlongster]

Wow, nice hack! We actually just merged in support for relative includes yesterday: #349

I'm wary of the performance hit of tracking the raw value for all nodes.

The loader now does get the name of the "parent" template: https://github.com/mozilla/nunjucks/blob/master/src/loader.js#L22 You could do custom resolution if needed there.

[boutell]

LOL! Love it. Thanks. I'll look and see if this can be adapted to our
slightly-different-but-similar use case.

On Fri, Feb 20, 2015 at 2:36 PM, James Long [email protected]
wrote:

Wow, nice hack! We actually just merged in support for relative includes
yesterday: #349 #349

I'm wary of the performance hit of tracking the raw value for all nodes.

The loader now does not get the name of the "parent" template:
https://github.com/mozilla/nunjucks/blob/master/src/loader.js#L22 You
could do custom resolution if needed there.

—
Reply to this email directly or view it on GitHub
#375 (comment).

*THOMAS BOUTELL, *DEV & OPS
P'UNK AVENUE | (215) 755-1330 | punkave.com

[boutell]

Hmm, this is super close, but since resolve is only called if the path
definitely starts with ./ or ../, it's not useful for the case where
template name resolution is not based directly on filesystem paths.

What I really want is to always have this opportunity to fix up the name
based on the parentName according to my own criteria. This allows for
module-based schemes like this:

blog:templatename.html
events:templatename.html

To work in conjunction with "relative" paths with no module name:

included.html

I would think that folks who are writing database-backed template loaders
would want this as well.

I think it makes sense to leave the interpretation of paths up to the
implementors of loaders.

If I modify this:

            // Resolve name relative to parentName

            if (parentName && (name.indexOf("./") == 0 ||

                               name.indexOf("../") == 0)) {

                name = loader.resolve(parentName, name);

            }

To just this:

            // Resolve name relative to parentName
            if (parentName) {
                name = loader.resolve(parentName, name);
            }

Then everything works great for my use case.

On Fri, Feb 20, 2015 at 2:59 PM, Tom Boutell [email protected] wrote:

LOL! Love it. Thanks. I'll look and see if this can be adapted to our
slightly-different-but-similar use case.

On Fri, Feb 20, 2015 at 2:36 PM, James Long [email protected]
wrote:

Wow, nice hack! We actually just merged in support for relative includes
yesterday: #349 #349

I'm wary of the performance hit of tracking the raw value for all nodes.

The loader now does not get the name of the "parent" template:
https://github.com/mozilla/nunjucks/blob/master/src/loader.js#L22 You
could do custom resolution if needed there.

—
Reply to this email directly or view it on GitHub
#375 (comment).

*THOMAS BOUTELL, *DEV & OPS
P'UNK AVENUE | (215) 755-1330 | punkave.com

*THOMAS BOUTELL, *DEV & OPS
P'UNK AVENUE | (215) 755-1330 | punkave.com

[boutell]

cc: @SamyPesse. I'm wishing that "resolve" were called at all times when parentName is set. Right now the "./" and "../" syntax, which doesn't really mean anything outside of a filesystem-based template loader, is being imposed awkwardly on database-driven and module-namespace-oriented loaders.

[SamyPesse]

We can add a method isRelative to the loader , but always resolving when parentName is present is a better solution.

[boutell]

I agree that either would cover the need.