Accept relative paths in import/include

[SamyPesse]

This PR fixes #228,#326, #243. It simply lets you use ../ and ./ in import and include.

• Pass path of current template to env.getTemplate
• Add tests

Example:

- folder1/
    - test.html
    - test2.html
- layout.html

test.html:

{% extends ../layout.html %}

{% block test %}
Hello
{% endblock %}

test2.html:

{% extends ./test.html %}

{% block test %}
Hello 2
{% endblock %}

In another matter, I don't think this line https://github.com/mozilla/nunjucks/blob/master/src/environment.js#L142 is a good idea, because if you're using an async loader inside your express application, the app crash if the loader return an error in the callback. The error should be passed to the callback (or throwned if sync): line 163.

[garygreen]

RE: line 142. I've been meaning to accept a PR for this #306, should fix this issue. Thanks for the PR. Does this work with async web loading the templates?

[SamyPesse]

I just added tests for relative paths: 4869a58

Feel free to let me know if my tests are not following your guidelines.

I don't know for async web loading, is there any tests for it? The most of the logic is here: https://github.com/mozilla/nunjucks/pull/349/files#diff-b77b8f7ef4365a2872db79644add92f8R121
When env.getTemplate has a parentName argument, it resolve the current path to it using path.resolve.

One improvement could be to transform this path.resolve into a method from the loader (which is defaulting to path.resolve), so that loader can handle its own way the resolve of the path. I'll add this, then it'll be ready to be merged 🚀

I'm not an english native, so I'm sorry if my messages are not clear.

[SamyPesse]

Ok, it should be good, I added a resolvemethod to the Loader object (https://github.com/SamyPesse/nunjucks/commit/927f2a6f5ec9b28ee2983ab146f469e557230683), so that Loader can resolve in their own way relative paths.

[jlongster]

This is a much requested feature, so thanks for digging in and making a PR! Your approach looks good. I'm not convinced that we need to push the resolve method into loaders; paths are the definitive way to describe templates in the system, and nunjucks assumes that you define templates as paths (because it maps well for nested templates). Even if you loaded your templates from a db, for examples, you would still key off paths and relative resolution would still work normally. It looks like the way you did it, if you request ./foo.html from app/nested/bar.html, it would resolve to the full path app/nested/foo.html and then get that template from the loader.

That approach seems fine to me and I would just hardcode that part of the resolution (at least for now). It would still work fine for loading templates over the web, as well as precompiled templates, because when it actually asks for the template it's using the full path.

[SamyPesse]

Ok, I move the resolve into the loader because path.resolve can have some side effects (it returns an absolute url, ...) and I'm not sure if it will work if all use cases (ex: I don't know how browserify is handling path.resolve).
I can remove my last commit, just let me know.

[SamyPesse]

I just added an option to the env.renderString method to accept an option to define the path to use for resolving paths. By default, this option is null.

I added a test for this.

[SamyPesse]

Any news ? :)

[fabien]

+1

[SamyPesse]

Do you want me to rebase the PR or you want to close it?

[jlongster]

Sorry, if you don't mind rebasing, that would be great!

[SamyPesse]

Done! It passed the tests

[jlongster]

Thanks! I'm pretty slammed at work right now but I'm definitely going to keep going through the PRs this week.

[jlongster]

Neat, because it resolves the path at runtime, it seems to work pretty well. Precompiled templates still work normally, which I thought there might be problem with.

Thanks a lot for this! Sorry I didn't get around to looking through it until now. I'll release a new version soon.

[SamyPesse]

Thanks 🍻

[rdebeasi]

Any plans to release this feature in a future version? Thanks! :)

[carljm]

It's already in multiple released versions.

[rdebeasi]

Whoops - looks like I was using an npm package that pulled in an outdated version of nunjucks. Sorry for the confusion!

[electerious]

Does this still work? I can't find a test for ../ and

<!-- button.njk -->
{% extends ../layout/layout.njk %}

{% block test %}
Hello
{% endblock %}

isn't working for me. It says

Template render error: (/Users/tobiasreich/Sites/Templates/Skeleton NJK/src/assets/components/button/button.njk)
 Error: template not found: /Users/tobiasreich/Sites/Templates/Skeleton NJK/src/assets/components/layout/layout.njk

but the template exists. FileSystemLoader searchPath points to /Users/tobiasreich/Sites/Templates/Skeleton NJK/src/assets/components/button/.

I'm using env.renderString and I'm passing a path to it to the file it renders.

// filePath === '/Users/tobiasreich/Sites/Templates/Skeleton NJK/src/assets/components/button/button.njk'
env.renderString(str, data, {
	path: filePath
}, next)

[electerious]

It works when I add /Users/tobiasreich/Sites/Templates/Skeleton NJK/src/assets/components/ to the searchPath of the FileSystemLoader. Is this the correct solution?

[electerious]

My code seems to fail because ../layout/layout.njk is "outside the box". Is there anything I can do to allow extends/imports "outside the box"?