Repository: mozilla/nunjucks (js)

Issue:
[Feature Request] Resolver for NPM Modules
This has previously been raised as #1136 but is still an issue.

As previously raised:

> Suppose we have a package installed like this:
> 
> ```
> `npm i -S @example/cards`
> ```
> Now want to import from it like this within a template:
> 
> ```
> `{% import "@example/templates/birthday.html" as birthday %}`
> ```
> Could the nunjucks resolver implement NPMs [resolv alogrithm](https://nodejs.org/api/modules.html#modules_all_together)?


A potential solution has previously been suggested:
> `nunjucks.configure('node_modules');`
https://github.com/mozilla/nunjucks/issues/1136#issuecomment-421107441

However, as far as I know, this doesn't use standard node_modules resolution and so would not work under several conditions:

- using `npm link`
- having templates in peerDependencies, as the node_modules path could not be calculated relative to the current file
- where a project has module1 as a dependency, module1 uses nunjucks, which in turn has a dependency on module2 that contains templates. Depending on the dependency tree, the templates may live in project/node_modules/module2, or project/node_modules/module1/node_modules/module2
- when using [yarn pnp](https://github.com/yarnpkg/rfcs/pull/101), node_modules does not exist at a known location on disk
- when using [npm tink](https://blog.npmjs.org/post/178027064160/next-generation-package-management)
- in a monorepo (e.g. using lerna or yarn workspaces), which introduces several other patterns such as linking between modules

Pull request:
Add NodeResolveLoader
## Summary

Adds `NodeResolveLoader`, a Loader that loads templates using node's [`require.resolve`](https://nodejs.org/api/modules.html#modules_all_together).

Fixes #1175

## Checklist

* [x] Proposed change helps towards [*purpose of this project*](https://github.com/mozilla/nunjucks/blob/master/CONTRIBUTING.md#purpose).
* [x] [*Documentation*](https://github.com/mozilla/nunjucks/tree/master/docs/) is added / updated to describe proposed change.
* [x] [*Tests*](https://github.com/mozilla/nunjucks/tree/master/tests) are added / updated to cover proposed change.
* [x] [*Changelog*](https://github.com/mozilla/nunjucks/blob/master/CHANGELOG.md) has an entry for proposed change (if user-facing fix or feature).

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5f44151..1aa975d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,6 +1,10 @@
 Changelog
 =========
 
+* Adds [`NodeResolveLoader`](http://mozilla.github.io/nunjucks/api.html#noderesolveloader),
+  a Loader that loads templates using node's
+  [`require.resolve`](https://nodejs.org/api/modules.html#modules_all_together).
+  Fixes [#1175](https://github.com/mozilla/nunjucks/issues/1175).
 * Emit 'load' events on `Environment` instances, to allow runtime dependency
   tracking. Fixes [#1153](https://github.com/mozilla/nunjucks/issues/1153).
 
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index 225fdf6..8b892c7 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -28,7 +28,7 @@ Issues and pull requests contributing to this purpose have the best chance to ma
 ## Questions?
 
 Please DO NOT ask "how do I?" or usage questions via GitHub issues. Instead,
-use the [https://groups.google.com/forum/#!forum/nunjucks](mailing list).
+use the [mailing list](https://groups.google.com/forum/#!forum/nunjucks).
 
 
 ## Submitting Issues
diff --git a/docs/api.md b/docs/api.md
index 8c516a5..b021847 100644
--- a/docs/api.md
+++ b/docs/api.md
@@ -193,6 +193,12 @@ use the simple [`configure`](#configure) API, nunjucks automatically
 creates the appropriate loader for you, depending if you're in node or
 the browser. See [`Loader`](#loader) for more information.
 
+Also only in node, [`NodeResolveLoader`](#noderesolveloader) is
+provided to allow templates to be included using
+[node `require` resolution](https://nodejs.org/api/modules.html#modules_all_together).
+This is not enabled by default with [`configure`](#configure), it must be
+explicitly passed into the `Environment` constructor.
+
 ```js
 // the FileSystemLoader is available if in node
 var env = new nunjucks.Environment(new nunjucks.FileSystemLoader('views'));
@@ -443,6 +449,18 @@ var env = new nunjucks.Environment(new nunjucks.FileSystemLoader('views'));
 
 {% endapi %}
 
+{% api %}
+NodeResolveLoader
+new NodeResolveLoader([opts])
+
+As the name suggests, this is also only available in node. It will load
+templates from the filesystem using node's
+[`require.resolve`](https://nodejs.org/api/modules.html#modules_all_together).
+
+**opts** is an object which takes the same properties as
+[`FileSystemLoader`](#filesystemloader).
+{% endapi %}
+
 {% api %}
 WebLoader
 new WebLoader([baseURL], [opts])
diff --git a/docs/fr/api.md b/docs/fr/api.md
index babac4b..5893a1b 100644
--- a/docs/fr/api.md
+++ b/docs/fr/api.md
@@ -182,6 +182,13 @@ utilisez l'API de configuration simplifiée, nunjucks crée  pour vous
 automatiquement le chargeur approprié, selon si vous êtes dans node ou dans
 le navigateur. Voir [`Chargeur`](#chargeur) pour plus d'informations.
 
+Aussi dans node, le [`NodeResolveLoader`](#noderesolveloader) est disponible
+pour charger depuis le système de fichiers selon l'algorithme de résolution
+du module node, ce qui est fait par
+[`require.resolve`](https://nodejs.org/api/modules.html#modules_all_together).
+Cet chargeur n'est pas activé par défaut; il faut passer éxplicitement au
+constructeur de `Environment`.
+
 ```js
 // Le FileSystemLoader est disponible si on est dans node
 var env = new nunjucks.Environment(new nunjucks.FileSystemLoader('views'));
@@ -431,6 +438,19 @@ var env = new nunjucks.Environment(new nunjucks.FileSystemLoader('views'));
 
 {% endapi %}
 
+{% api %}
+NodeResolveLoader
+new NodeResolveLoader([opts])
+
+Comme le nom le suggère, cet chargeur n'est disponible que dans node.
+Il chargera les templates depuis le système de fichiers selon l'algorithme de
+résolution du module node, ce qui est fait par
+[`require.resolve`](https://nodejs.org/api/modules.html#modules_all_together).
+
+**opts** est un object avec les même propriétés que
+[`FileSystemLoader`](#filesystemloader).
+{% endapi %}
+
 {% api %}
 WebLoader
 new WebLoader([baseURL], [opts])
diff --git a/docs/templating.md b/docs/templating.md
index 73a0a75..9f4698e 100644
--- a/docs/templating.md
+++ b/docs/templating.md
@@ -926,8 +926,9 @@ Return a list of lists with the given number of items:
 
 ```jinja
 {% set items = [1,2,3,4,5,6] %}
+{% set dash = joiner("-") %}
 {% for item in items | batch(2) %}
-    -{% for items in item %}
+    {{ dash() }} {% for items in item %}
        {{ items }}
     {% endfor %}
 {% endfor %}
diff --git a/nunjucks/index.js b/nunjucks/index.js
index 2544ef9..25d4d48 100644
--- a/nunjucks/index.js
+++ b/nunjucks/index.js
@@ -49,6 +49,7 @@ module.exports = {
   Template: Template,
   Loader: Loader,
   FileSystemLoader: loaders.FileSystemLoader,
+  NodeResolveLoader: loaders.NodeResolveLoader,
   PrecompiledLoader: loaders.PrecompiledLoader,
   WebLoader: loaders.WebLoader,
   compiler: compiler,
diff --git a/nunjucks/src/node-loaders.js b/nunjucks/src/node-loaders.js
index 42405a0..16147ca 100644
--- a/nunjucks/src/node-loaders.js
+++ b/nunjucks/src/node-loaders.js
@@ -86,7 +86,64 @@ class FileSystemLoader extends Loader {
   }
 }
 
+class NodeResolveLoader extends Loader {
+  constructor(opts) {
+    super();
+    opts = opts || {};
+    this.pathsToNames = {};
+    this.noCache = !!opts.noCache;
+
+    if (opts.watch) {
+      if (!chokidar) {
+        throw new Error('watch requires chokidar to be installed');
+      }
+      this.watcher = chokidar.watch();
+
+      this.watcher.on('change', (fullname) => {
+        this.emit('update', this.pathsToNames[fullname], fullname);
+      });
+      this.watcher.on('error', (error) => {
+        console.log('Watcher error: ' + error);
+      });
+
+      this.on('load', (name, source) => {
+        this.watcher.add(source.path);
+      });
+    }
+  }
+
+  getSource(name) {
+    // Don't allow file-system traversal
+    if ((/^\.?\.?(\/|\\)/).test(name)) {
+      return null;
+    }
+    if ((/^[A-Z]:/).test(name)) {
+      return null;
+    }
+
+    let fullpath;
+
+    try {
+      fullpath = require.resolve(name);
+    } catch (e) {
+      return null;
+    }
+
+    this.pathsToNames[fullpath] = name;
+
+    const source = {
+      src: fs.readFileSync(fullpath, 'utf-8'),
+      path: fullpath,
+      noCache: this.noCache,
+    };
+
+    this.emit('load', name, source);
+    return source;
+  }
+}
+
 module.exports = {
   FileSystemLoader: FileSystemLoader,
-  PrecompiledLoader: PrecompiledLoader
+  PrecompiledLoader: PrecompiledLoader,
+  NodeResolveLoader: NodeResolveLoader,
 };
diff --git a/package.json b/package.json
index f3a6a13..9cc0a60 100644
--- a/package.json
+++ b/package.json
@@ -73,7 +73,7 @@
     "lint": "eslint nunjucks scripts tests",
     "prepare": "npm run build",
     "test:instrument": "cross-env NODE_ENV=test scripts/bundle.js",
-    "test:runner": "cross-env NODE_ENV=test scripts/testrunner.js",
+    "test:runner": "cross-env NODE_ENV=test NODE_PATH=tests/test-node-pkgs scripts/testrunner.js",
     "test": "npm run lint && npm run test:instrument && npm run test:runner"
   },
   "bin": {


Test patch:
diff --git a/tests/loader.js b/tests/loader.js
index c3b9a58..0968377 100644
--- a/tests/loader.js
+++ b/tests/loader.js
@@ -5,6 +5,7 @@
     Environment,
     WebLoader,
     FileSystemLoader,
+    NodeResolveLoader,
     templatesPath;
 
   if (typeof require !== 'undefined') {
@@ -12,12 +13,14 @@
     Environment = require('../nunjucks/src/environment').Environment;
     WebLoader = require('../nunjucks/src/web-loaders').WebLoader;
     FileSystemLoader = require('../nunjucks/src/node-loaders').FileSystemLoader;
+    NodeResolveLoader = require('../nunjucks/src/node-loaders').NodeResolveLoader;
     templatesPath = 'tests/templates';
   } else {
     expect = window.expect;
     Environment = nunjucks.Environment;
     WebLoader = nunjucks.WebLoader;
     FileSystemLoader = nunjucks.FileSystemLoader;
+    NodeResolveLoader = nunjucks.NodeResolveLoader;
     templatesPath = '../templates';
   }
 
@@ -111,5 +114,43 @@
         });
       });
     }
+
+    if (typeof NodeResolveLoader !== 'undefined') {
+      describe('NodeResolveLoader', function() {
+        it('should have default opts', function() {
+          var loader = new NodeResolveLoader();
+          expect(loader).to.be.a(NodeResolveLoader);
+          expect(loader.noCache).to.be(false);
+        });
+
+        it('should emit a "load" event', function(done) {
+          var loader = new NodeResolveLoader();
+          loader.on('load', function(name, source) {
+            expect(name).to.equal('dummy-pkg/simple-template.html');
+            done();
+          });
+
+          loader.getSource('dummy-pkg/simple-template.html');
+        });
+
+        it('should render templates', function() {
+          var env = new Environment(new NodeResolveLoader());
+          var tmpl = env.getTemplate('dummy-pkg/simple-template.html');
+          expect(tmpl.render({foo: 'foo'})).to.be('foo');
+        });
+
+        it('should not allow directory traversal', function() {
+          var loader = new NodeResolveLoader();
+          var dummyPkgPath = require.resolve('dummy-pkg/simple-template.html');
+          expect(loader.getSource(dummyPkgPath)).to.be(null);
+        });
+
+        it('should return null if no match', function() {
+          var loader = new NodeResolveLoader();
+          var tmplName = 'dummy-pkg/does-not-exist.html';
+          expect(loader.getSource(tmplName)).to.be(null);
+        });
+      });
+    }
   });
 }());
diff --git a/tests/test-node-pkgs/dummy-pkg/index.js b/tests/test-node-pkgs/dummy-pkg/index.js
new file mode 100644
index 0000000..e69de29
diff --git a/tests/test-node-pkgs/dummy-pkg/package.json b/tests/test-node-pkgs/dummy-pkg/package.json
new file mode 100644
index 0000000..20651d5
--- /dev/null
+++ b/tests/test-node-pkgs/dummy-pkg/package.json
@@ -0,0 +1,12 @@
+{
+  "name": "dummy-pkg",
+  "version": "1.0.0",
+  "description": "",
+  "main": "index.js",
+  "scripts": {
+    "test": "echo \"Error: no test specified\" && exit 1"
+  },
+  "keywords": [],
+  "author": "Frankie Dintino <fdintino@gmail.com> (http://www.frankiedintino.com/)",
+  "license": "ISC"
+}
diff --git a/tests/test-node-pkgs/dummy-pkg/simple-template.html b/tests/test-node-pkgs/dummy-pkg/simple-template.html
new file mode 100644
index 0000000..008c714
--- /dev/null
+++ b/tests/test-node-pkgs/dummy-pkg/simple-template.html
@@ -0,0 +1,1 @@
+{{ foo }}
\ No newline at end of file