-
-
Notifications
You must be signed in to change notification settings - Fork 54
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
statsByDimensionsSync generates wrong src on first run #146
Comments
Alright, I was able to lean on sanity's on-demand image processing to generate the HTML for a responsive image in a synchronous fashion: const client = require('../sanity.js'); // fully configured sanity client, see https://www.sanity.io/docs/js-client
const imageUrlBuilder = require('@sanity/image-url');
const builder = imageUrlBuilder(client);
module.exports = function sanityImage({ source, alt, aspect_ratio, sizes = '100w', loading = 'lazy', decoding = 'async' }) {
if (alt === undefined) throw new Error('Alt has to be specified');
// get essential data from asset
const {
metadata: {
dimensions: {
width: _w,
aspectRatio: _aspect
}
}
} = source.asset;
// define widths for output
let widths = [320, 400, 700, 1200, 2000];
// filter out widths that are smaller than the actual image
widths = widths.filter(w => w <= _w);
// specify output formats and corresponding mimetype
const formats = {
webp:'image/webp',
jpg: 'image/jpeg'
};
// generate sources for each format and width
const sources = Object.keys(formats).map(f => {
const _sources = widths.map(w => {
let url, h;
if (aspect_ratio !== undefined) {
// custom aspect ratio defined --> crop image
const [aspect_w, aspect_h] = aspect_ratio.split('x');
const dec_aspect_ratio = aspect_w / aspect_h;
h = Math.round(w / dec_aspect_ratio);
url = builder.image(source).width(w).height(h).format(f).url();
} else {
// no custom aspect ratio, just specify width
h = Math.round(w / _aspect)
url = builder.image(source).width(w).format(f).url();
}
return {
url,
w,
h
}
});
return {
format: f,
mimetype: formats[f],
srcsets: _sources.map(_s => `${_s.url} ${_s.w}w`).join(', '),
srcs: _sources
}
});
// get last item of `sources` (jpeg) and select first item of `srcs`, which is the smallest size
const img = sources[sources.length - 1].srcs[0];
// generate HTML
const html = `
<picture>
${sources.map(_s => {
return `
<source
type="${_s.mimetype}"
srcset="${_s.srcsets}"
sizes="${sizes}"
/>
`
}).join('')}
<img
alt="${alt}"
loading="${loading}"
decoding="${decoding}"
src="${img.url}"
width="${img.w}"
height="${img.h}"
/>
</picture>`;
return html;
} |
Is the image remote? The filename should be generated by this function: Line 294 in d9141cb
It reads the contents of the file, and hashes them with SHA256. Usually. Line 314 in d9141cb
That's a place where the hash could change. Can you see if this bug is reproduceable? If you could make a repository that reproduces this bug, I'll debug it and possibly draft a pull request to fix it. |
@zeroby0 Yes, the image is remote. The bug should be reproducible – I'll share some code with in the next few days, if you wan't to debug it 🙌🏼 |
@zeroby0 I finally took some time trying to make a repo to reproduce the bug. Appearently I was using v1.0.0 of the img package. It works with v2.0.0. https://github.com/timonforrer/11ty-statsByDimensionsSync/ I'll close this issue |
Hi again, @zeroby0 I was a bit hasty.. Bumping the version to v2.0.0 on the repository, that was originally affected by the bug, didn't resolve the issue. I added a layout and generate the pages using the pagination feature on the reproduction repo. Now the behaviour is as originally described – the img src is wrong on the first build. https://github.com/timonforrer/11ty-statsByDimensionsSync/ I would be very glad, if you could have a look at it. Thank you! |
Heyo @timonforrer Don't worry, it happens :) Your repo was very useful in tracing the bug, and it's from this line: Line 314 in d9141cb
There is a race condition. If When running for the first time, In the second run, the url is already in the cache, and This makes the hash change and return a different src the first time. |
@zeroby0 thank you for looking into it and creating the PR! |
Thanks for reporting the bug @timonforrer 😄 |
Hello @zeroby0 , I have the exact same issue described by @timonforrer . Thanks |
@Olivier-Rasquin I don't know either, I'm eager for it to be reviewed and merged!
@Olivier-Rasquin meanwhile, you can use npm r eleventy-img
npm i zeroby0/eleventy-img#issue-146 --save-dev to temporarily replace eleventy-img with the pull request version. UpdateYou should probably trigger builds twice instead of using this fork. I'm not sure that completely solves all the cache bugs either. Please see the discussion in the accompanies PR for more details. |
Thanks @zeroby0 ! We've just decided that we will wait for the official "update", and in the meantime we are triggering two successive builds. Let's hope it can be reviewed fairly soon! |
I hope so too! Good luck! |
I'm using sanity studio and their portable text with a serializer, to turn the portable text into html.
The serializer is a shortcode, that looks like this:
And the
generateImage()
function is:I'm using
Image.statsByDimensionsSync()
so that I don`t have to use async – I tried the async way before, but that didn't work in conjunction with the sanity serializer.I found that
Image.statsByDimensionsSync()
generates a differentsrc
on the first run. So when I just build the site, the generated html for the image tries to load a file that doesn't exist. When I use eleventy serve and let eleventy rebuild the site once, it works fine.I assume that eleventy generates the filename based on a different value on the first run than on subsequent runs.
E.g. on the first run the src is
/images/CWhDmUxMUw-320.jpeg
, on subsequent runs it's/images/Zzo8VdfzGM-320.jpeg
The text was updated successfully, but these errors were encountered: