Open tropperstyle opened 8 years ago
patrick-steele-idem
commented
8 years ago Hey @tropperstyle, the code appears to be buggy in that it doesn't check for the err argument in the callback function:
Could you update the function body locally to see what the err is and let me know? In the meantime I will try to reproduce.
Thanks.
patrick-steele-idem
commented
8 years ago Hey @tropperstyle, I wasn't able to reproduce using the following setup:
node_modules/foo/template.marko:
<lasso-img src="./logo.png"/>Compiled template:
node_modules/foo/template.marko.js:
function create(__helpers) {
var str = __helpers.s,
empty = __helpers.e,
notEmpty = __helpers.ne,
__logo_png = require.resolve("./logo.png"),
__getImageInfo = require("lasso/taglib/helper-getImageInfo"),
attr = __helpers.a;
return function render(data, out) {
__getImageInfo(out, __logo_png, function(out, imageInfo1) {
out.w('<img' +
attr("src", imageInfo1.url) +
attr("width", imageInfo1.width) +
attr("height", imageInfo1.height) +
'>');
});
};
}
(module.exports = require("marko").c(__filename)).c(create);Notice that the ./logo.png path gets resolved using the following code:
__logo_png = require.resolve("./logo.png")Everything looks correct to me.
tropperstyle
commented
8 years ago Ah, the difference in my use case is that the src attribute is interpolated:
<lasso-img src="./img/${img}.png"/>This prevents the variable from becoming hoisted and is instead inlined as:
__getImageInfo(out, "./img/"+img+".png", function(out, imageInfo1) {
out.w('<img' +
attr("src", imageInfo1.url) +
attr("width", imageInfo1.width) +
attr("height", imageInfo1.height) +
'>');
});@tropperstyle The simple fix is to make sure you pass the fully resolved path to your template. I also recommend creating a lookup for all of the possible images (along with the fully resolved path):
var images = {
"image1": require.resolve('./img/image1.png'),
"image2": require.resolve('./img/image2.png'),
};And then just pass the appropriate fully resolved path to the template. With that approach your code will work on both the server and in the browser (Lasso.js needs to know which image info objects to send to the browser).
Does that work for you?
tropperstyle
commented
8 years ago Wouldn't the root fix be to alter the output of the compiled template? Something like:
__getImageInfo(out, require.resolve("./img/"+img+".png"), function(out, imageInfo1) { ...I prefer passing only data to the template, and isolating presentation logic to view helpers. Specifically, I am passing in a list of country codes. I could expand that to a country object which contains an attribute with the fully resolved path, but I'd prefer my original workaround which at least stays in the view:
<lasso-img src="${__dirname}/flags/${code}.png" for="code in data.countryCodes"/>But if I understand correctly, it sounds like this approach has issues with marko-widgets? This particular template is not rendered in the client, but still good to know.
patrick-steele-idem
commented
8 years ago Wouldn't the root fix be to alter the output of the compiled template? Something like:
Yes, but require.resolve is actually pretty slow (even though it is cached). We could add require.resolve if we detect that the image path is dynamic, but... It wouldn't allow the same code to work on the server since Lasso.js wouldn't know to include the image info for a dynamic path like that.
Your workaround is good as long as you are okay with the code only working on the server.
A potential fit seems to lie around lasso-img-tag-transformer.js#L14
But I tried wrapping the second argument passed to setAttribute with
require('path').resolve(template.dirname, ...)and the way that failed leaves me questioning if this line is really the right entry point.Current workaround:
<lasso-img src="./relative/path.png"/>=><lasso-img src="${__dirname}/relative/path.png"/>