Extensionless extends, includes and imports

[ArmorDarks]

Is there any way to provide nunjucks with templates files extension name, so we could drop it in extends, includes and imports.

For example, Instead of

{% extends "layout.nj" %}

use

{% extends "layout" %}

I've heard that it's possible to configure extension name when Nunjucks used in conjuration with Express. But what about other use cases, when Nunjucks used on it's own, or with Grunt?

[jlongster]

Expresses imposes a weird relationship with a templating engine, as it wants to do these kinds of things itself. I'm not against supporting it natively though. If you would like to add it, open a PR.

[carljm]

Closing this issue due to lack of follow-up. I think it's low priority, and it doesn't exist in Jinja2.

[ArmorDarks]

:( I hopped that there is easy way to hook up it from Express-only to regular Nunjucks, but, unfortunately, didn't have enough time to look into it.

[ArmorDarks]

@carljm posted in #691 (comment):

I still don't like #475 -- I think the recommended extension should remain a matter only for the documentation and the community, not something that has an actual effect on the behavior. Even if it were configurable, I still don't see the benefit -- it makes your includes etc correspond less clearly to the actual template files, why is that a good thing?

It's very minor thing, I agree. But it's just somehow wrong to ask developer specify extension every single time while importing or including something when you're clearly working in unified environment.

I think closest example here is ES2016, babel, systemjs. When you're importing other modules, you do not specify .js extension, unless you really need to specify extension for some serious reason. For example, when importing non-standard for system file. In other words, not .js one, but Sass or CSS. or image. It's just a good design, since system arguably assumes that you by default would require specific to current environment files only, and not anything else. In some sense, it DRYing out code, by removing constantly repeating and really not useful extension.

Same principle used in many other modern systems.

Option to drop extension just makes Nunjucks more unified with them.

[carljm]

I think nunjucks already provides the flexibility necessary to implement this, though; it would just require a custom loader.

[ArmorDarks]

Just to ensure — you don't see it as part of Nunjucks core? Otherwise, would be better to reopen issue.

[carljm]

No, I don't see it as part of Nunjucks core. I can see why someone might want it, but I've also worked on plenty of projects where templates didn't use a consistent dedicated extension, but rather just the extension of their ultimate type (.html, .txt, etc). I prefer keeping Nunjucks simple and completely extension-agnostic, and allowing anyone who wants this feature to provide it via a custom loader.

[ArmorDarks]

Sure, you're the boss :)

I prefer keeping Nunjucks simple and completely extension-agnostic

That had to be extension-agnostic. You should be able to specify your extension or set of extensions during configuring environment. Besides, it shouldn't affect imports and includes, which already had hardcoded extensions.

and allowing anyone who wants this feature to provide it via a custom loader

True, but more loaders and deviations from vendor aren't good thing too.

So, I think for now that issue should stay dead.

[ArmorDarks]

@carljm I've played around with Nunjucks internals and loaders, and came up with that solution:

https://github.com/mozilla/nunjucks/blob/master/src/node-loaders.js#L54:

    getSource: function(name) {
        ....

        for(var i=0; i<paths.length; i++) {
            var basePath = path.resolve(paths[i]);
            var p = path.resolve(paths[i], name);

            // Here is where magic happens
            // We add extension only if it wasn't specified
            if (path.extname(p) != '.nj') {
              p += '.nj'
            }

            ....
        }

        ....
});

Now {% include 'paste' %} instead of {% include 'paste.nj' %} will work.

I wonder, is basic approach here okay?

And besides I wonder, is there any way to extend getSource() method completely and then extend it? So far it seems that if we will use custom loader, we will have to reimplement getSource() method. It will effectively break, if one day Nunjucks will decide to commit some changes to internals and node-loader...

I also somehow dislike, that in fact here we're hijacking all incoming filenames, even for base layouts. In other words, it makes index (extensionless) filename valid one, while we want it to be valid only for includes, imports and extends. But, seems that loader doesn't have way to tell is he returning current src and path for template rendering, or for include, or import, or extend.

The only way so far to do it only for includes, imports and extends is monkeypatching https://github.com/mozilla/nunjucks/blob/master/src/parser.js#L429

    parseInclude: function() {
        var tagName = 'include';
        var tag = this.peekToken();
        if(!this.skipSymbol(tagName)) {
            this.fail('parseInclude: expected '+ tagName);
        }

        var node = new nodes.Include(tag.lineno, tag.colno);
        node.template = this.parseExpression();
        node.template.value += '.nj'; // <--------- Here is crude example

        if(this.skipSymbol('ignore') && this.skipSymbol('missing')) {
            node.ignoreMissing = true;
        }

        this.advanceAfterBlockEnd(tag.value);
        return node;
    },

Am I missing something?

[carljm]

I apparently didn't understand the details of the feature that you wanted, and I'm afraid I don't understand why you want it that way. What is the benefit of having includes, extends and imports have optional extensions, while extensions are required for the original render? That seems to me like a recipe for confusion. Whatever the rules are for referencing a template in a given environment, those rules should be consistent for everywhere templates are referenced.

[ArmorDarks]

Just for the clarification, I don't think that feature is vitally needed, but I've already tried to explain that it will make imports, includes and extends more consistent with similar entities from close environments.

Node.js requires:

var Something = require('fileName')

ES2016 imports:

import Something from fileName

Python imports:

from fileName import Something

It doesn't seem to make systems more confusing.

[carljm]

But the difference is that in all of those environments, you always leave off the extension. What I'm not understanding is why you want to include the extension when calling render, but leave it off when using include, extend, or import. If you just want to always leave it off, that version I think can be implemented as a custom loader, right?

[ArmorDarks]

But the difference is that in all of those environments, you always leave off the extension.

Not always. You will need to specify extension, if you requiring or importing something, that isn't specific for that environment.

For example, with SystemJS loader:

import './style.scss!'

You need to specify extension here, because it isn't specific for current environment file, it's custom one, so system can't know what you want to import here. It can make assumptions about index.js, *.js, even *.coffee in some situation, but not about unknown extension.

What I'm not understanding is why you want to include the extension when calling render, but leave it off when using include, extend, or import.

Few paragraphs above is the reason. For Nunjucks, where includes, imports and extends executed, our predefined extensions (.njk, .nj, .html or whatever) are common and known extension. Nunjucks system knows about them, and what is very important, they are native for Nunjucks. So, when you say {% extends 'base' %} it's logical to assume that base is base.njk, or base.html.

But for JavaScript those extensions aren't native. render() executed not in Nunjucks environment, but in JavaScript's one, for which .js is known and native extension, while .njk isn't native. So, technically, render('index') is confusing, because it's executed in JS environment and it's unclear does it refer to index.nj or index.js.

If you just want to always leave it off

Well, after giving it a thought, even despite what I've said above, I think it's ok. Not something I would expect in render(), but that might be nice side-effect too.

I think can be implemented as a custom loader, right?

Yes, it's possible. But I've already mentioned, that overriding getSource() is definitely bad thing. It would be better if we could have at least possibility to manipulate it's values with callbacks, without full override.

But to be honest, I'd insist it to be part of default loader. It is very simple thing, but it will unify imports, includes and extends behavior with Node.js and ES2016 imports behavior, which is something you might expect as part of JS ecosystem and good design.

To summarize, what technically I want here to implement, is make templates loading much more intelligent and predictable, like it's done in Node.js.

Crude examples:

// In environment configuration settings we set `ext` to be `['.njk', '.nj', '.html']`
{% include 'base' %}

Will look in following order for:

1. base
2. base.njk
3. base.nj
4. base.html

{% include 'base.njk' %}

Will look in following order for:

1. base.njk
2. base.njk.njk
3. base.njk.nj
4. base.njk.html

{% include 'base/' %}

Will look in following order for:

1. base/index
2. base/index.njk
3. base/index.nj
4. base/index.html

Otherwise it would be great at least to have access to getSource() internals with callbacks without need to fully override that method.

[carljm]

I hear your arguments, but I am afraid I am still not convinced of the value of having this in core. I just don't think the parallel to the JS or Python environment holds, mostly because the use of certain well-known extensions for Nunjucks templates is barely even a convention yet, whereas in JS and Python it is required.

Basically, if you want this in core Nunjucks, you're going to have to get it into Jinja2 first.

Or maybe come back to it in a year or so, once the use of a standard common extension for Nunjucks templates is much more strongly established.

In the meantime, I see your issue with overriding getSource; I don't have time to dig into it deeply, but I would have no problem with a PR that refactors some internals to make it easier to extend a built-in loader and customize name munging. If you submit such a PR, I'd be happy to review and merge it.

[ArmorDarks]

mostly because the use of certain well-known extensions for Nunjucks templates is barely even a convention yet

Or maybe come back to it in a year or so, once the use of a standard common extension for Nunjucks templates is much more strongly established.

I do not propose to predefine out of box any extension in Nunjucks.

I propose to make configuration during environment setup possible out of box.

So, it's independent from whether we have established extension or no. It's more question of do we want to have such feature or no.

In other words, we need to look for other reasons to deny or approve such addition.

Basically, if you want this in core Nunjucks, you're going to have to get it into Jinja2 first.

I might be wrong, but according to what I've researched regarding Jinja2, there is nothing to get in Jinja2 first.

Like Nunjucks, Jinja2 supports different loaders out of box, as well as custom loaders. So, basically, you can do anything you want there, and not rarely it more dependent on by which framework it's used. For example, if I'm not mistaken, usage of Django with Django-Jinja allows to set those extensions in configuration.

Some might arise question about compatibility of such loader with Jinja2. I've thought about it a while. But seems that it is one of those domains, where we can't maintain literal compatibility with Jinja2. Nunjucks has different (in future) extension, it's loading environment is different too, some applied in Python practices aren't applicable in JS and vica-versa, so how could we guarantee here that default Nunjucks loaders, which are for JS and Node, will behave exactly like Jinja2 loaders for Python and Django?

In the meantime, I see your issue with overriding getSource; I don't have time to dig into it deeply, but I would have no problem with a PR that refactors some internals to make it easier to extend a built-in loader and customize name munging. If you submit such a PR, I'd be happy to review and merge it.

If such changes to extends, imports and includes, will be forbidden for eternity, I will try to figure out good solution for this. Otherwise, it seems to be unneeded for most cases, and will only complicate API

[carljm]

Yes, I understand that you intend to make it configurable, with no default. But I think this is precisely why it is not the same as JS or Python; because there is no standard assumed extension throughout the entire ecosystem. So when you see an extension-less nunjucks import, you have to refer back to the environment configuration (and in some cases that may not be trivial to find, especially for a template author who perhaps didn't set up the environment code themselves) to have a clue what the actual filename might be. Now, for working on a particular project, that's something you would get used to. But in terms of the overall nunjucks ecosystem, it makes it harder, not easier, to come into a new project and quickly find your away around the templates.

On the level of the entire language ecosystem, it is an absolute given (a requirement, practically) that a JS file will have the extension js and a Python file will have the extension .py, so it is reasonable to elide those extensions without losing any clarity at all; there is only one possibility for what the extension might be, no matter whose Python or JS files you are reading. You don't have to look up some project-specific configuration somewhere to figure that out. There simply is no parallel here to the nunjucks ecosystem. Your proposed configurability is precisely what makes your proposal fundamentally different from the situation with JS or Python.

I personally frequently use git grep something.njk to find other templates including or extending that template (especially when entering a new project for the first time). If this feature were added, I would have to be aware that I must now leave off the extension when grepping, or I may miss some uses. And leaving off the extension when grepping may cause other problems, something may be a common word in the project.

It is true that in Jinja you could write a custom template loader that would allow eliding the extension (though I've never personally seen one); I would be happy to make this equally possible in Nunjucks.

Jinja does not include out of the box any loader that allows eliding template extensions; similarly I do not wish to include such a loader out of the box in Nunjucks.

(Your django-jinja example is unrelated; the extensions you configure there are for the purpose of allowing Django's generic template rendering API to decide which template engine should be used to render a given template, based on its extension. You still have to provide the full filename, including extension, in all imports/extends/includes.)

[rightaway]

You can leave off the extension when pointing to a template and it will take the value of opts.defaultEngine as the default extension to use right? So this same default extension mechanism should be allowed for extends, from, etc. Otherwise you'll be pointing to a mypage template (without an extension), but that will extends 'parent.njk' (with the extension).

[carljm]

defaultEngine is not documented, and it's not a feature of nunjucks core, only of the express integration.

[ArmorDarks]

@carljm Sorry for delay, still don't have enough time to fully reply on your previous message.

Just an interesting additional info — ConsolidateJS seems to use extensionless templates too, and it doesn't seem like anybody suffering from it:

// set .html as the default extension 
app.set('view engine', 'html');
app.set('views', __dirname + '/views');

app.get('/', function(req, res){
  res.render('index', {
    title: 'Consolidate.js'
  });
});

app.get('/users', function(req, res){
  res.render('users', {
    title: 'Users',
    users: users
  });
});

[rightaway]

+1 since .njk is now the recommended extension, it would help adoption of it if .njk would be automatically appended where there is no extension provided.

[dashtinejad]

@ArmorDarks @carljm Any progress on this?