inject a templateEngine param to the template adapter

rendrjs / rendr

Render your Backbone.js apps on the client and the server, using Node.js.

MIT License

4.09k stars 312 forks source link

inject a templateEngine param to the template adapter #467

Closed ingiulio closed 9 years ago

ingiulio commented 9 years ago

Add a template system to the defaults, with default value handlebars to go along with the default templateAdapter: 'rendr-handlebars'. This would allow us to directly control the version of Handlebars or of the template system that we are using from the Rendr app, instead of being coupled to what's inside the template adapter module.

This needs another PR in rendr-handlebars to be merged: https://github.com/rendrjs/rendr-handlebars/pull/52

Thoughts?

coveralls commented 9 years ago

Coverage remained the same at 84.22% when pulling b160cbc59c741f2ba2fb0c7fbff7db0cb588a983 on ingiulio:injectTemplateSystemToTemplateAdapter into 5ada76ac4d284008e966037c7e0d86b83190f326 on rendrjs:master.

alexindigo commented 9 years ago

@rendrjs/maintainers Would like to have your input on both PRs (satellite PR – https://github.com/rendrjs/rendr-handlebars/issues/52) Thank you.

alexindigo commented 9 years ago

:+1:

alexindigo commented 9 years ago

@ingiulio another idea is to pass templateEngine as second argument, to make it backwards compatible solution and allow template adapter to decide on accepting external template engine or using bundled solution.

Example:

this.templateAdapter = require(templateAdapterModule)(templateAdapterOptions, templateEngine);

ingiulio commented 9 years ago

@alexindigo I like that idea a lot. I'll change this as you suggested.

coveralls commented 9 years ago

Coverage increased (+0.28%) to 84.5% when pulling 79d7918621411ca7145607b015111180e8e85e4c on ingiulio:injectTemplateSystemToTemplateAdapter into 5ada76ac4d284008e966037c7e0d86b83190f326 on rendrjs:master.

coveralls commented 9 years ago

Coverage increased (+0.28%) to 84.5% when pulling 8eb46e1a61caf48fef63839e07c4109ebc1296ed on ingiulio:injectTemplateSystemToTemplateAdapter into 5ada76ac4d284008e966037c7e0d86b83190f326 on rendrjs:master.

pjanuario commented 9 years ago

:+1:

coveralls commented 9 years ago

Coverage increased (+0.28%) to 84.5% when pulling 3d9de1a3b45144a15c62bc9b661dfc75f1209215 on ingiulio:injectTemplateSystemToTemplateAdapter into 5ada76ac4d284008e966037c7e0d86b83190f326 on rendrjs:master.

spikebrehm commented 9 years ago

LGTM. There's a merge conflict now -- care to rebase?

saponifi3d commented 9 years ago

@spikebrehm you're alive?! haha. :)

I agree about the rebase - once we get this PR in I'll do a final test run through and then release 1.0.4 today :dancers:

ingiulio commented 9 years ago

@spikebrehm @saponifi3d PR updated. Thank you.

coveralls commented 9 years ago

Coverage remained the same at 84.5% when pulling 84a7ddd29b23bc1d0b47dc6703943c2bd88bb4ef on ingiulio:injectTemplateSystemToTemplateAdapter into e312db824ddfef0eda364b201c9774eb663b727a on rendrjs:master.

ingiulio commented 9 years ago

@saponifi3d just a reminder: this one will need https://github.com/rendrjs/rendr-handlebars/pull/52 merged too