search

vitkarpov / grunt-nunjucks-2-html

Compiles nunjucks templates *right* into HTML
MIT License
35 stars 11 forks source link

Pass options to Nunjucks directly #50

Closed ArmorDarks closed 7 years ago

ArmorDarks commented 7 years ago

There is no reason to cherry pick only certain options which will be passed from this.options() to nunjucks.configure().

Instead, we can brutally pass all options directly to nunjucks.configure(). Yes, sometimes it will also pass not relevant to nunjucks options like configureEnvironment and so on, but since Nunjucks doesn't know anything about them, they will be simply ignored.

This change will allows us to rest assured that users will be always able to pass environment configurations to Nunjucks, no matter what API changes will happen in Nunjucks.

Also, it will allow users to change options they wanted to change. For instance, right now we've hardcoded noChache: true and users wasn't able to enable cache whenever they needed it.

vitkarpov commented 7 years ago

I thought, maybe, it should be specific option like nunjucksOptions, but it breaks current API

ArmorDarks commented 7 years ago

Yeap, I hesitate around that thought too.

ArmorDarks commented 7 years ago

Btw, can we merge this into 2.x branch too?

vitkarpov commented 7 years ago

Maybe, not doing this is a good way to move people forward :) Are you sure this is a must have feature?

ArmorDarks commented 7 years ago

There is a critical bug in Nunjucks 3.x due to which we can't move from 2.x. It hasn't been fixed for almost a year... But we'd like to enable cache in 2.x.

Well, I think it's more like a personal requirement, since not much people will encounter that bug, so I'll probably just make temporary fork of master and lower version to 2.x

vitkarpov commented 7 years ago

I think you can make a pr into v2.x, I'll merge it.

ср, 14 июня 2017 г. в 14:41, Serj Lavrin notifications@github.com:

There is a critical bug in Nunjucks 3.x due to which we can't move from 2.x. It hasn't been fixed for almost a year... But we'd like to enable cache in 2.x.

Well, I think it's more like a personal requirement, since not much people will encounter that bug, so I'll probably just make temporary fork of master and lower version to 2.x

— You are receiving this because you modified the open/close state.

Reply to this email directly, view it on GitHub https://github.com/vitkarpov/grunt-nunjucks-2-html/pull/50#issuecomment-308405919, or mute the thread https://github.com/notifications/unsubscribe-auth/AC3Nv_1EyXqi9wH2WqvKiHNd5pVHtM-Tks5sD8bWgaJpZM4N4ghT .

vitkarpov commented 7 years ago

I would do it by myself if I had the feature branch in this repo, but it's in your fork, I'm sorry.