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
configuration layoutsDir
& partialsDir
is invalid
#244
configuration layoutsDir
& partialsDir
is invalid
#244
Comments
Yeah, I also found it. express-handlebars/lib/express-handlebars.js Lines 24 to 32 in 5d27bb5
so I think maybe change like this is fine for global custom one or default one utils.assign(this, {
handlebars : Handlebars,
extname : '.handlebars',
// layoutsDir : 'views/layouts/',
// partialsDir : 'views/partials/',
defaultLayout : undefined,
helpers : undefined,
compilerOptions: undefined,
}, config); and if (viewsPath) {
view = this._getTemplateName(path.relative(viewsPath, viewPath));
this.partialsDir = this.partialsDir || path.join(viewsPath, 'partials/');
this.layoutsDir = this.layoutsDir || path.join(viewsPath, 'layouts/');
} I changed on my local like this and it work fine with my old settings |
At least until the following issue gets resolved: ericf/express-handlebars#244
At least until the following issue gets resolved: ericf/express-handlebars#244
we don't need to comment here: utils.assign(this, {
handlebars : Handlebars,
extname : '.handlebars',
// layoutsDir : 'views/layouts/',
// partialsDir : 'views/partials/',
defaultLayout : undefined,
helpers : undefined,
compilerOptions: undefined,
}, config); because the function The problem really resides here: // Express provides `settings.views` which is the path to the views dir that
// the developer set on the Express app. When this value exists, it's used
// to compute the view's name. Layouts and Partials directories are relative
// to `settings.view` path
var view;
var viewsPath = options.settings && options.settings.views;
if (viewsPath) {
view = this._getTemplateName(path.relative(viewsPath, viewPath));
this.partialsDir = path.join(viewsPath, 'partials/');
this.layoutsDir = path.join(viewsPath, 'layouts/');
} Why the We should not in any case if if (viewsPath) {
view = this._getTemplateName(path.relative(viewsPath, viewPath));
if(Array.isArray(this.partialsDir)) {
this.partialsDir.push(path.join(viewsPath, 'partials/'));
} else {
this.partialsDir = [path.join(viewsPath, 'partials/')];
}
if(Array.isArray(this.layoutsDir)) {
this.layoutsDir.push(path.join(viewsPath, 'layouts/'));
} else {
this.layoutsDir = [path.join(viewsPath, 'layouts/')];
}
} Maybe it could be more compact, but I think it keeps the structure. I tested and it works |
@jfoclpf Thanks for replying. First, why I need to comment out the definition in constructor is to deal with the pass-through settings defined outside, eg. this.partialsDir = this.partialsDir || path.join(viewsPath, 'partials/');
this.layoutsDir = this.layoutsDir || path.join(viewsPath, 'layouts/'); Second, for your provided modification, why you MUST push this path At end, I don't know why you think |
@dogbutcat var utils = require('./utils');
function ExpressHandlebars(config) {
// Config properties with defaults.
utils.assign(this, {
handlebars : Handlebars,
extname : '.handlebars',
layoutsDir : 'views/layouts/',
partialsDir : 'views/partials/',
defaultLayout : undefined,
helpers : undefined,
compilerOptions: undefined,
}, config);
exports.assign = Object.assign || require('object.assign'); which I suppose it clones the object, and thus if you defined your partials at
I'm not saying you can't, I am just trying to make the module as fully compatible as possible, and the folder
I know, but the internal function var partialsDirs = Array.isArray(this.partialsDir) ? this.partialsDir : [this.partialsDir]; because it may be defined as an array: var hbs = exphbs.create({
extname: '.hbs',
partialsDir: [
path.join(__dirname, 'views', 'common'),
path.join(__dirname, 'views', 'main'),
path.join(__dirname, 'css', 'merged-min'),
path.join(__dirname, 'tables')
],
helpers: hbsHelpers
}) Therefore I assumed that probably it's better to always considered it internally as an array. But you're right, maybe it's superfluous. |
@ericf just replace this if (viewsPath) {
view = this._getTemplateName(path.relative(viewsPath, viewPath));
this.partialsDir = path.join(viewsPath, 'partials/');
this.layoutsDir = path.join(viewsPath, 'layouts/');
} by either this if (viewsPath) {
view = this._getTemplateName(path.relative(viewsPath, viewPath));
this.partialsDir = this.partialsDir || path.join(viewsPath, 'partials/');
this.layoutsDir = this.layoutsDir || path.join(viewsPath, 'layouts/');
} or this if (viewsPath) {
view = this._getTemplateName(path.relative(viewsPath, viewPath));
if(Array.isArray(this.partialsDir)) {
this.partialsDir.push(path.join(viewsPath, 'partials/'));
} else {
this.partialsDir = [this.partialsDir || path.join(viewsPath, 'partials/')];
}
if(Array.isArray(this.layoutsDir)) {
this.layoutsDir.push(path.join(viewsPath, 'layouts/'));
} else {
this.layoutsDir = [this.partialsDir || path.join(viewsPath, 'layouts/')];
}
} Thank you |
Thank you for reporting the issue. The bug has been fixed and the new version v3.0.2 is now published on npm. |
After 3.0.1 was released, the configuration options
layoutsDir
andpartialsDir
became invalid.Because after Fixes #147, the custom configuration options
layoutsDir
andpartialsDir
will be overridden.I hope it can be repaired as soon as possible.Thank you.
The text was updated successfully, but these errors were encountered: