Skip to content
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

Is there any plan to seperate options from locals? #309

Open
wxfred opened this issue Apr 17, 2018 · 1 comment
Open

Is there any plan to seperate options from locals? #309

wxfred opened this issue Apr 17, 2018 · 1 comment

Comments

@wxfred
Copy link

wxfred commented Apr 17, 2018

In lib/consolidate.js, there are many render functions use locals as options to build a template, then again, transfer the locals to the template for constructing the final page. So, users may ignore the potential issue that their locals could be used as options for the engine. And, this shortcut is not recommend by ejs.

However, be aware that your code could break if we add an option with the same name as one of your data object's properties. Therefore, we do not recommend using this shortcut. -- from https://www.npmjs.com/package/ejs

I think changing the interface from
(path[, locals], callback)
to
(path[, locals, options], callback)
will be a good solution.

@doowb
Copy link
Collaborator

doowb commented Apr 17, 2018

This has come up before and I think it's a good idea too. I'd like to do some refactoring of the code and I'll keep this in mind when doing that. One thing that needs to be thought about is how it impacts other libraries, like express, that expect template engines to have the current api. This probably just requires the correct version bump, then other libraries can choose to update to use the new api or work around it.

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

No branches or pull requests

2 participants