-
Notifications
You must be signed in to change notification settings - Fork 122
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
oc-template-jade module #375
Conversation
5367fb3
to
5ef251b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple of little things
src/cli/domain/package-template.js
Outdated
compileDebug: false, | ||
name: 't' | ||
}).toString().replace('function t(locals) {', 'function(locals){'); | ||
preCompiledView = jade.precompile(template, { filename: viewPath }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I would like to do something similar to the oc-client here rather than all the ifs. Something like
var precompilers = { jade, handlebars};
// and then
if(!precompilers[type]){
throw strings.errors.cli.TEMPLATE_TYPE_NOT_VALID;
}
var preCompiledView = precompilers[type].precompile(template, { viewPath });
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, note that in jade we named that viewPath, not filename 👍
var context = { jade: require('jade/runtime.js')}; | ||
vm.runInNewContext(templateText, context); | ||
template = context.oc.components[key]; | ||
template = jade.getPrecompiledTemplate(templateText, key); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as before here.
Updated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, only thing missing is the dep on the client/package.json. Sorry I missed it on my prev review
Should be fixed now |
Done per jade what we did for handlebars in #371
To be rerun once oc-template-jade will be published