diff --git a/.changeset/brown-pumpkins-clean.md b/.changeset/brown-pumpkins-clean.md
new file mode 100644
index 000000000000..f45c791ed0f1
--- /dev/null
+++ b/.changeset/brown-pumpkins-clean.md
@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Fix duplicate images being created in some cases when using densities and/or widths
diff --git a/.changeset/serious-news-yell.md b/.changeset/serious-news-yell.md
new file mode 100644
index 000000000000..b110464258cb
--- /dev/null
+++ b/.changeset/serious-news-yell.md
@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+fix remote srcset images not being resized
diff --git a/packages/astro/components/Picture.astro b/packages/astro/components/Picture.astro
index 1be6371d16a1..f6c7686e8790 100644
--- a/packages/astro/components/Picture.astro
+++ b/packages/astro/components/Picture.astro
@@ -57,14 +57,13 @@ if (fallbackImage.srcSet.values.length > 0) {
{
- Object.entries(optimizedImages).map(([_, image]) => (
- 0 ? ' , ' + image.srcSet.attribute : ''
- }`}
- type={'image/' + image.options.format}
- />
- ))
+ Object.entries(optimizedImages).map(([_, image]) => {
+ const srcsetAttribute =
+ props.densities || (!props.densities && !props.widths)
+ ? `${image.src}${image.srcSet.values.length > 0 ? ', ' + image.srcSet.attribute : ''}`
+ : image.srcSet.attribute;
+ return ;
+ })
}
diff --git a/packages/astro/package.json b/packages/astro/package.json
index 510365a44fac..cb11269355d8 100644
--- a/packages/astro/package.json
+++ b/packages/astro/package.json
@@ -139,6 +139,7 @@
"common-ancestor-path": "^1.0.1",
"cookie": "^0.5.0",
"debug": "^4.3.4",
+ "deterministic-object-hash": "^1.3.1",
"devalue": "^4.3.2",
"diff": "^5.1.0",
"es-module-lexer": "^1.3.0",
diff --git a/packages/astro/src/assets/services/service.ts b/packages/astro/src/assets/services/service.ts
index 455531dfd753..7d2f6afb9098 100644
--- a/packages/astro/src/assets/services/service.ts
+++ b/packages/astro/src/assets/services/service.ts
@@ -213,6 +213,10 @@ export const baseService: Omit = {
options.format = DEFAULT_OUTPUT_FORMAT;
}
+ // Sometimes users will pass number generated from division, which can result in floating point numbers
+ if (options.width) options.width = Math.round(options.width);
+ if (options.height) options.height = Math.round(options.height);
+
return options;
},
getHTMLAttributes(options) {
@@ -230,16 +234,33 @@ export const baseService: Omit = {
},
getSrcSet(options) {
const srcSet: SrcSetValue[] = [];
- const { targetWidth, targetHeight } = getTargetDimensions(options);
+ const { targetWidth } = getTargetDimensions(options);
const { widths, densities } = options;
const targetFormat = options.format ?? DEFAULT_OUTPUT_FORMAT;
- const aspectRatio = targetWidth / targetHeight;
- const imageWidth = isESMImportedImage(options.src) ? options.src.width : options.width;
- const maxWidth = imageWidth ?? Infinity;
+ // For remote images, we don't know the original image's dimensions, so we cannot know the maximum width
+ // It is ultimately the user's responsibility to make sure they don't request images larger than the original
+ let imageWidth = options.width;
+ let maxWidth = Infinity;
+
+ // However, if it's an imported image, we can use the original image's width as a maximum width
+ if (isESMImportedImage(options.src)) {
+ imageWidth = options.src.width;
+ maxWidth = imageWidth;
+ }
- // REFACTOR: Could we merge these two blocks?
+ // Since `widths` and `densities` ultimately control the width and height of the image,
+ // we don't want the dimensions the user specified, we'll create those ourselves.
+ const {
+ width: transformWidth,
+ height: transformHeight,
+ ...transformWithoutDimensions
+ } = options;
+
+ // Collect widths to generate from specified densities or widths
+ const allWidths: { maxTargetWidth: number; descriptor: `${number}x` | `${number}w` }[] = [];
if (densities) {
+ // Densities can either be specified as numbers, or descriptors (ex: '1x'), we'll convert them all to numbers
const densityValues = densities.map((density) => {
if (typeof density === 'number') {
return density;
@@ -248,64 +269,49 @@ export const baseService: Omit = {
}
});
+ // Calculate the widths for each density, rounding to avoid floats.
const densityWidths = densityValues
.sort()
.map((density) => Math.round(targetWidth * density));
- densityWidths.forEach((width, index) => {
- const maxTargetWidth = Math.min(width, maxWidth);
-
- // If the user passed dimensions, we don't want to add it to the srcset
- const { width: transformWidth, height: transformHeight, ...rest } = options;
-
- const srcSetValue = {
- transform: {
- ...rest,
- },
- descriptor: `${densityValues[index]}x`,
- attributes: {
- type: `image/${targetFormat}`,
- },
- };
-
- // Only set width and height if they are different from the original image, to avoid duplicated final images
- if (maxTargetWidth !== imageWidth) {
- srcSetValue.transform.width = maxTargetWidth;
- srcSetValue.transform.height = Math.round(maxTargetWidth / aspectRatio);
- }
-
- if (targetFormat !== options.format) {
- srcSetValue.transform.format = targetFormat;
- }
-
- srcSet.push(srcSetValue);
- });
+ allWidths.push(
+ ...densityWidths.map((width, index) => ({
+ maxTargetWidth: Math.min(width, maxWidth),
+ descriptor: `${densityValues[index]}x` as const,
+ }))
+ );
} else if (widths) {
- widths.forEach((width) => {
- const maxTargetWidth = Math.min(width, maxWidth);
-
- const { width: transformWidth, height: transformHeight, ...rest } = options;
-
- const srcSetValue = {
- transform: {
- ...rest,
- },
- descriptor: `${width}w`,
- attributes: {
- type: `image/${targetFormat}`,
- },
- };
-
- if (maxTargetWidth !== imageWidth) {
- srcSetValue.transform.width = maxTargetWidth;
- srcSetValue.transform.height = Math.round(maxTargetWidth / aspectRatio);
- }
+ allWidths.push(
+ ...widths.map((width) => ({
+ maxTargetWidth: Math.min(width, maxWidth),
+ descriptor: `${width}w` as const,
+ }))
+ );
+ }
- if (targetFormat !== options.format) {
- srcSetValue.transform.format = targetFormat;
+ // Caution: The logic below is a bit tricky, as we need to make sure we don't generate the same image multiple times
+ // When making changes, make sure to test with different combinations of local/remote images widths, densities, and dimensions etc.
+ for (const { maxTargetWidth, descriptor } of allWidths) {
+ const srcSetTransform: ImageTransform = { ...transformWithoutDimensions };
+
+ // Only set the width if it's different from the original image's width, to avoid generating the same image multiple times
+ if (maxTargetWidth !== imageWidth) {
+ srcSetTransform.width = maxTargetWidth;
+ } else {
+ // If the width is the same as the original image's width, and we have both dimensions, it probably means
+ // it's a remote image, so we'll use the user's specified dimensions to avoid recreating the original image unnecessarily
+ if (options.width && options.height) {
+ srcSetTransform.width = options.width;
+ srcSetTransform.height = options.height;
}
+ }
- srcSet.push(srcSetValue);
+ srcSet.push({
+ transform: srcSetTransform,
+ descriptor,
+ attributes: {
+ type: `image/${targetFormat}`,
+ },
});
}
@@ -356,6 +362,17 @@ export const baseService: Omit = {
},
};
+/**
+ * Returns the final dimensions of an image based on the user's options.
+ *
+ * For local images:
+ * - If the user specified both width and height, we'll use those.
+ * - If the user specified only one of them, we'll use the original image's aspect ratio to calculate the other.
+ * - If the user didn't specify either, we'll use the original image's dimensions.
+ *
+ * For remote images:
+ * - Widths and heights are always required, so we'll use the user's specified width and height.
+ */
function getTargetDimensions(options: ImageTransform) {
let targetWidth = options.width;
let targetHeight = options.height;
diff --git a/packages/astro/src/assets/utils/transformToPath.ts b/packages/astro/src/assets/utils/transformToPath.ts
index d9e11f3222fe..82c5cc2793e3 100644
--- a/packages/astro/src/assets/utils/transformToPath.ts
+++ b/packages/astro/src/assets/utils/transformToPath.ts
@@ -1,3 +1,4 @@
+import { deterministicString } from 'deterministic-object-hash';
import { basename, extname } from 'node:path';
import { removeQueryString } from '../../core/path.js';
import { shorthash } from '../../runtime/server/shorthash.js';
@@ -19,5 +20,5 @@ export function hashTransform(transform: ImageTransform, imageService: string) {
// Extract the fields we want to hash
const { alt, class: className, style, widths, densities, ...rest } = transform;
const hashFields = { ...rest, imageService };
- return shorthash(JSON.stringify(hashFields));
+ return shorthash(deterministicString(hashFields));
}
diff --git a/packages/astro/test/core-image.test.js b/packages/astro/test/core-image.test.js
index 8e007edb4d0f..ade9799186b6 100644
--- a/packages/astro/test/core-image.test.js
+++ b/packages/astro/test/core-image.test.js
@@ -225,7 +225,61 @@ describe('astro:image', () => {
const srcset2 = parseSrcset($source.attr('srcset'));
expect(srcset2.every((src) => src.url.startsWith('/_image'))).to.equal(true);
- expect(srcset2.map((src) => src.w)).to.deep.equal([undefined, 207]);
+ expect(srcset2.map((src) => src.w)).to.deep.equal([207]);
+ });
+
+ it('properly deduplicate srcset images', async () => {
+ let res = await fixture.fetch('/srcset');
+ let html = await res.text();
+ $ = cheerio.load(html);
+
+ let localImage = $('#local-3-images img');
+ expect(
+ new Set([
+ ...parseSrcset(localImage.attr('srcset')).map((src) => src.url),
+ localImage.attr('src'),
+ ]).size
+ ).to.equal(3);
+
+ let remoteImage = $('#remote-3-images img');
+ expect(
+ new Set([
+ ...parseSrcset(remoteImage.attr('srcset')).map((src) => src.url),
+ remoteImage.attr('src'),
+ ]).size
+ ).to.equal(3);
+
+ let local1x = $('#local-1x img');
+ expect(
+ new Set([
+ ...parseSrcset(local1x.attr('srcset')).map((src) => src.url),
+ local1x.attr('src'),
+ ]).size
+ ).to.equal(1);
+
+ let remote1x = $('#remote-1x img');
+ expect(
+ new Set([
+ ...parseSrcset(remote1x.attr('srcset')).map((src) => src.url),
+ remote1x.attr('src'),
+ ]).size
+ ).to.equal(1);
+
+ let local2Widths = $('#local-2-widths img');
+ expect(
+ new Set([
+ ...parseSrcset(local2Widths.attr('srcset')).map((src) => src.url),
+ local2Widths.attr('src'),
+ ]).size
+ ).to.equal(2);
+
+ let remote2Widths = $('#remote-2-widths img');
+ expect(
+ new Set([
+ ...parseSrcset(remote2Widths.attr('srcset')).map((src) => src.url),
+ remote2Widths.attr('src'),
+ ]).size
+ ).to.equal(2);
});
});
diff --git a/packages/astro/test/fixtures/core-image/src/pages/srcset.astro b/packages/astro/test/fixtures/core-image/src/pages/srcset.astro
new file mode 100644
index 000000000000..221b1b7c4a00
--- /dev/null
+++ b/packages/astro/test/fixtures/core-image/src/pages/srcset.astro
@@ -0,0 +1,59 @@
+---
+import { Image } from "astro:assets";
+import image from "../assets/penguin2.jpg";
+
+---
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
index a6f0cedd9338..ef5773e5e189 100644
--- a/pnpm-lock.yaml
+++ b/pnpm-lock.yaml
@@ -541,6 +541,9 @@ importers:
debug:
specifier: ^4.3.4
version: 4.3.4(supports-color@8.1.1)
+ deterministic-object-hash:
+ specifier: ^1.3.1
+ version: 1.3.1
devalue:
specifier: ^4.3.2
version: 4.3.2
@@ -10372,6 +10375,10 @@ packages:
requiresBuild: true
dev: false
+ /deterministic-object-hash@1.3.1:
+ resolution: {integrity: sha512-kQDIieBUreEgY+akq0N7o4FzZCr27dPG1xr3wq267vPwDlSXQ3UMcBXHqTGUBaM/5WDS1jwTYjxRhUzHeuiAvw==}
+ dev: false
+
/devalue@4.3.2:
resolution: {integrity: sha512-KqFl6pOgOW+Y6wJgu80rHpo2/3H07vr8ntR9rkkFIRETewbf5GaYYcakYfiKz89K+sLsuPkQIZaXDMjUObZwWg==}