search

tj / consolidate.js

Template engine consolidation library for node.js
3.48k stars 357 forks source link

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

Open wxfred opened 6 years ago

wxfred commented 6 years ago

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 commented 6 years ago

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.