sindresorhus / gulp-nunjucks

Precompile Nunjucks templates
MIT License
152 stars 20 forks source link

Wrong file name in output when multiple templates are processed #1

Closed ingro closed 10 years ago

ingro commented 10 years ago

Hello, first of all thanks for this and your other plugins for Gulp!

Anyway I found an issue while working with multiple templates, here's an example code:

gulp.task('templates', function() {
    gulp.src(['public/templates/**/*.html'])
    .pipe(nunjucks())
    .pipe(concat('templates.js'))
    .pipe(gulp.dest('public/scripts/build'));
});

If for example I have this templates in my target folder:

posts/ ->list.html ->single.html admin.html main.html

in the output javascript file they are all present but all named as 'admin.html' and so not usable in the browser.

Looking at the source code I suppose it assign the first file's path to the name attribute in the option hash and so all the subsequent templates will have the same name, since they share the same option variable.

I hope that I managed to explain the problem well enough, sorry for my bad english :(

sindresorhus commented 10 years ago

Yeah, I see that now. Not sure what I was thinking.

Possible solution:

Default to file.relative. Eg app/templates/template for a file located in app/templates/template.html. Would that be ok, or would you prefer just template?

For the naming it could be a callback where you can create the name for every file yourself:

gulp.task('templates', function() {
    gulp.src(['public/templates/**/*.html'])
    .pipe(nunjucks({
        name: function (file) {
            return 'tpl-' + path.basename(file.relative, path.extname(file.relative));
        }
    }))
    .pipe(concat('templates.js'))
    .pipe(gulp.dest('public/scripts/build'));
});

Thoughts?

ingro commented 10 years ago

I like the solution of using a callback to manually create the name, it gives us full control on that.

For the default I'd like if it could strip the root folder so we can have a relative name to the folder used to configure nunjucks in node, for example:

nunjucks.configure('public/templates',
{
    express: app
});

So I can call my template's rendering like this:

res.render('posts/list.html');

and the partials, include and extends function will work too.

Maybe you could also accept a folder instead of a list of files as input?

sindresorhus commented 10 years ago

For the default I'd like if it could strip the root folder so we can have a relative name to the folder used to configure nunjucks in node, for example:

Alright, cool, that's what I suggested.

I also prefer to remove the extension as you would never need to have different extensions with the same relative path.

Maybe you could also accept a folder instead of a list of files as input?

No, gulp works on files.

ingro commented 10 years ago

Ah ok, I don't have much experience with Gulp yet :P Anyway I'm not sure about removing the extension from the name, in this pull request nunjuck's author suggested to leave it...

jlongster commented 10 years ago

I merged that PR. Let me know if there's anything else you think we should add to nunjucks.

sindresorhus commented 10 years ago

@ingro can you try out master? does the readme look ok to you?

ingro commented 10 years ago

Yay, now it works fine even with the default options, also the readme looks good to me!

Thank you for the fast fix :D