allow absolute path to be passed to this.render #34
Conversation
|
@jwilm ping! |
| @@ -156,6 +156,12 @@ Hbs.prototype.createRenderer = function() { | |||
| var tplPath = path.join(hbs.viewPath, tpl + hbs.extname), | |||
| template, rawTemplate, layoutTemplate; | |||
|
|
|||
| // if the template passed has any non-word characters leading, | |||
| // we're probably trying to pass an absolute path, so roll with that. | |||
| if (!fs.existsSync(tplPath) && !/^\w/.test(tpl)) { | |||
There was a problem hiding this comment.
This won't work for windows style absolute paths C:\Path. Combining the regex test with an exists check is a bit odd as well. I think it might be better to refactor the tplPath generation into a function which inspects the received tpl for leading / or /^[a-zA-Z]:\//. I guess we can check if path.isAbsolute is defined for node 12.x series and then fallback to such a regex implementation for older versions.
|
Seems like a fine feature. I left some suggestions about the implemenation. Thanks for the PR! |
|
I'm not sure how to interpret that response :) The PR is fully functional for our org. We're using it in a production environment. If you feel it can be improved upon, that's kinda on you as the project's sole collaborator. Letting the PR wither would be denying other users a useful feature. We'll probably roll with our fork until this is merged in. |
It's on me to maintain code quality in the library, ensure the supported features work as they are supposed to, work with third party contributors to get their features added if they make sense in the context of the project, and occasionally add features myself. What's not my responsibility is finishing any contributor's half-baked feature which does not support all platforms (which koa/node support) and which does not have tests for the new functionality. If you want to see this feature added to the library, you can work with me to get it added in a way that will work for everybody. Alternatively, you can submit a feature request and I will get to it eventually. |
|
That's fair I suppose. You can consider this the feature request if you'd like :) (I feel it's a bit harsh to say the PR is half-baked, could have done with different language there. We're all just trying to improve the product) |
|
@shellscape You're totally right. The half-baked comment was not meant to be derogatory in any way. Sorry for that. Let's just call it a feature request as you have suggested. |
|
Sounds good man, thanks much! |
|
please comment if you have a vested interest. this PR also bumps the minimum version of node to 4.x. There have now been 3 major version releases of node since 0.12, and there's no sense in supporting a platform that old. |
|
left this change up for a month without comment, so here goes... |
We have several situations where handlers are being defined in other modules, but we wish to take advantage of the rendering pipeline that koa-hbs provides. This is currently not possible if the templates exist outside of the
viewPathlocation that's configured. As copying and creating symlinks aren't desirable for this application, allowing absolute paths to views is reasonable.