bump nunjucks?

[waffledonkey]

Can I talk you into bumping your Nunjucks dependency to 3.0.0

"nunjucks": "^3.0.0"

This is because ancient nunjucks needs ancient fsevents which no longer builds or I'd even be up for nunjucks as an optionalDependencies

Also, if you're feeling generous would you consider changing line 59 of the task to:

var environment = options.env || new nunjucks.Environment([fileLoader]);

I use a custom environment with filters like 'layout' and 'partial' so that I can do this sort of syntax:

{% extends 'base' | layout %}

versus having to have the complete relative path back to the layout in each template. You already support a custom env, but your only honoring it for pre-compiling not rendering.

[adiktofsugar]

Soooo...I didn't make any tests for this, don't use it personally anymore, and don't feel confident updating nunjucks 2 major versions.

However! I would like to help out! But I need you to send me a sample to test against, so I can make some tests that will let me rest easy.

So, if you'd like me to do this work, send me something I can test against. Specifically, something that nunjucks 1 doesn't work for but nunjucks 3 does.

Thanks for expressing interest!

[adiktofsugar]

also, why don't you use https://github.com/vitkarpov/grunt-nunjucks-2-html rather than this?

[waffledonkey]

It's not that it doesn't work with nunjucks 1, it's that nunjucks 1 has a dependency on chokadir 0.8.2 which has a dependency on fsevents 0.2.1 which fails to build. fsevents is an OS X specific file system events thing. nunjucks uses such tech to "watch", but in the case of building (either rendering or precompiling) there is no need to watch... basically any time I try to install any npm dependency, npm tries to (re)compile fsevents; and fails.

I can't imagine a use case where you'd run grunt-nunjucks-alt and not have nunjucks already, so moving nunjucks to an optional dependency or removing it completely seems like an easy fix (imo) -- especially if you don't already have tests that might use it. Also, not to pile on, but nunjucks 1 has an XSS vulnerability. I'm reminded of how connect-couchbase doesn't mention couchbase as a dependency at all -- presumably because it's a relatively safe assumption.

As for why I don't use grunt-nunjucks-2, it's because it seems to only render and I need both render and precompile. I could grunt-shell the precompile maybe, but it feels sub-optimal. Your module seems to be the only one that does both. Cheers for that!

I had been using grunt-nunjucks to precompile and grunter-render-nunjucks to render but the documentation for grunt-nunjucks is wrong and necessitated that I alter the task. Your module does exactly what I need and I could even go back to a more traditional extends syntax if need be. I'm not keen to do that, but I could. For now I've deleted node_modules/grunt-nunjucks-alt/node_modules, stripped the dependencies from your modules package.json and then hacked on package-lock.json to convince npm that those dependencies never existed. It works fine (for whatever that's worth), but I'm using my options.env and not allowing new nunjucks.Environment([fileLoader]). It's possible that method signature has changed from 1 to 3... I'll punt my env and see what happens. I don't know how I'd share it for you to create tests from, but I'm open to it.

Thank you for your time and consideration

[waffledonkey]

Ok I punted my custom env and this looks to be what's required.

57:     // actually set up the nunjucks stuff
58:     var fileLoader = new taskLoaders.FileSystemLoader( searchPaths, options.name, {baseDir: options.baseDir} );
59:     var environment = options.env || nunjucks.Environment;
60:
61:     grunt.log.debug("made environment", time());

Nunjucks 3 does not use Environment as a constructor and instead creates one for you automatically. fileLoader is called directly, not through the nunjucks object (nunjucks-alt:js: line 74), so there's no need to assign it to the nunjucks object.

[adiktofsugar]

Ok, i had to move and do some other things, and come up with tests and the like. I'm not sure what you're talking about nunjucks 3 not having an Environment constructor, because they definitely do, but I've made a few key changes.

You can't pass in an actual environment anymore because this plugin only works with the custom loader, so i made a beforeEnv option to let you add your filters and extensions and stuff in there.

nunjucks dependency is updated, and i made sure it works with grunt 1.x, so you're good there, too, if that was an issue.

I'll leave this open for a week, so let me know if these changes work in your environment. It's published as 1.0.0.

[waffledonkey]

Firstly, thank you very much! :::air five:::

I regret to report, however, that it is not working for me.

npm ls grunt-nunjucks-alt && grunt nunjucks-alt
redacted@1.0.0 /Users/redacted/Sites/localhost/
└── grunt-nunjucks-alt@1.0.0 

Running "nunjucks-alt:server" (nunjucks-alt) task
Warning: nunjucks.Environment is not a constructor Use --force to continue.

Aborted due to warnings.

I'd gotten that before under the previous version, when I deleted the local nunjucks (1.0.7) and chokadir (0.8.4) and relied on the existing nunjucks (3.1.2).

I was able to avoid that error by replacing this line: var environment = new nunjucks.Environment([fileLoader])

with this: var environment = options.env || nunjucks.Environment

I chose to approach it that way because I was getting the error that nunjucks.Environment was not a constructor AND you only ever call fileLoader statically (line 78) AND you already had a mechanism to externalize the env. With that change, I then just set the options.env to my env; which already had filters added.

beforeEnv is interesting, but I would have to refactor my nunjucks module (shown below). That's doable, of course, but this feels like more of a departure from what you had previously and I'm concerned there are people who might be affected by this change -- it also means more work for me which I'm selfishly conscious of. As is my module is called statically (replacing nunjucks.Environment with my own) and returns a nunjucks instance which I bind to Express or render through (asynchronously).

Something like this:

"use strict";

const nunjucks = require('nunjucks');

class Nunjucks {

    static init() {

        // returns an Environment (I believe this is the new constructor and thus the error)
        // nunjucks running in a server context assumes a FileLoader, just as nunjucks running on the 
        // client assumes the WebLoader
        //
        //                           views dir             {autoescape: true, etc.}
        var env = nunjucks.configure(config.express.views, config.nunjucks);

        env.addFilter...

        nunjucks.Environment = env;

        return nunjucks;
}

module.exports = Nunjucks.init();

[adiktofsugar]

nunjucks.Environment is supposed to be a constructor. See https://mozilla.github.io/nunjucks/api.html#environment.

Your code sample shows you setting nunjucks.Environment to the environment nunjucks.configure returns for you...don't do that, you're changing how nunjucks works at a basic level....

Refactoring to add the filters should be trivial, just make a module that exports a function like this:

module.exports = env => {
  env.addFilter...
}

Then use that filter in the grunt task and your static class.

I changed the way the env thing worked because it really does need that specific loader. I realized working on this that that's why it was only honored for precompiling. I think it's much simpler now and doesn't advertise it can do more than it really can.

[waffledonkey]

It does sort of work. I broke up my static class from one that returns nunjucks and one that does filter work which has some benefits to me anyway. The issue I face now is that while it does write a file, the file is empty.

You seem to be switching from nunjucks.precompile for pre-compiling and environment.render for rendering, but if I recall correctly the reason my original static class returned an instance of Nunjucks and not an instance of Environment was because environment didn't have a render method -- but I'm not taking any errors related to that it's just not producing any results. Err. It writes files, they're just empty.

[D] started file routes/homepage/index.html 42ms elapsed
[D] templateName for routes/homepage/index.html is homepage/index.html
[D] templateData { error: 
   { title: 'Error: dummy error',
     stack: 'at /path/to/foo.js:1:1\n at /path/to/bar.js:2:1' } }
[D] rendered 50ms elapsed
>> Wrote homepage/index.html as .tmp/routes/homepage/index.html

Done.

ls -l .tmp/routes/homepage/index.html
-rw-r--r--  1 redacted  staff  0 Apr 29 02:14 .tmp/routes/homepage/index.html

I have views alongside their corresponding routes (index.js; express.Router) inside of routes

routes
    _layouts
    _partials
    homepage
        index.js
        index.html
    error.html
    index.js

If I specify a searchPaths of routes it can find error.html but it's resulting content is empty and it cannot find homepage/index.html at all. if I specify searchpaths as routes/** it can find both error.html and homepage/index.html but the resulting files for both are empty. The same occurs if I don't specify searchPaths.

I guess I'm struggling with the correlation between cwd, baseDir, searchPaths and src. I don't use baseDir, but trying made no difference. My src is consistently: routes/**/*.html, I typically omit cwd -- though it varies wildly between '.', process.cwd() and 'routes' -- and changing srcPaths similarly varies wildly between whether it can find a thing or not. Usually it can find the template to be able to resolve it to a name, but then can't find the file to render it which doesn't make much sense to me because it found it to resolve it to a name... :(

While I have you, if we can use globbing in src and src can be an array, then why do we need searchPaths?