Skip to content

Pass options to Nunjucks directly #50

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 13, 2017

Conversation

ArmorDarks
Copy link
Contributor

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.

Copy link
Owner

@vitkarpov vitkarpov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉 👍

@vitkarpov vitkarpov merged commit ddc23f2 into vitkarpov:master Jun 13, 2017
@ArmorDarks ArmorDarks deleted the feature/all-options branch June 13, 2017 16:39
@vitkarpov
Copy link
Owner

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

@ArmorDarks
Copy link
Contributor Author

Yeap, I hesitate around that thought too.

@ArmorDarks
Copy link
Contributor Author

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

@vitkarpov
Copy link
Owner

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

@ArmorDarks
Copy link
Contributor Author

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
Copy link
Owner

vitkarpov commented Jun 14, 2017 via email

@vitkarpov
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants