-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
feat: experimental responsive images #12377
Changes from 13 commits
5f328e0
84492b0
ffe063c
e4a9c35
814b873
46055a6
fd68ea9
b500b10
582f997
f752102
d28ba03
a6b283a
b746c15
4889154
cc1db75
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
--- | ||
'astro': patch | ||
--- | ||
|
||
Adds experimental reponsive image support |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
[data-astro-image] { | ||
width: 100%; | ||
height: auto; | ||
object-fit: var(--fit); | ||
object-position: var(--pos); | ||
aspect-ratio: var(--w) / var(--h); | ||
} | ||
/* Styles for responsive layout */ | ||
[data-astro-image='responsive'] { | ||
max-width: calc(var(--w) * 1px); | ||
max-height: calc(var(--h) * 1px); | ||
} | ||
/* Styles for fixed layout */ | ||
[data-astro-image='fixed'] { | ||
width: calc(var(--w) * 1px); | ||
height: calc(var(--h) * 1px); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,12 @@ import { isRemotePath } from '@astrojs/internal-helpers/path'; | |
import { AstroError, AstroErrorData } from '../core/errors/index.js'; | ||
import type { AstroConfig } from '../types/public/config.js'; | ||
import { DEFAULT_HASH_PROPS } from './consts.js'; | ||
import { | ||
DEFAULT_RESOLUTIONS, | ||
LIMITED_RESOLUTIONS, | ||
getSizesAttribute, | ||
getWidths, | ||
} from './layout.js'; | ||
import { type ImageService, isLocalService } from './services/service.js'; | ||
import { | ||
type GetImageResult, | ||
|
@@ -32,9 +38,13 @@ export async function getConfiguredImageService(): Promise<ImageService> { | |
return globalThis.astroAsset.imageService; | ||
} | ||
|
||
type ImageConfig = AstroConfig['image'] & { | ||
experimentalResponsiveImages: boolean; | ||
}; | ||
|
||
export async function getImage( | ||
options: UnresolvedImageTransform, | ||
imageConfig: AstroConfig['image'], | ||
imageConfig: ImageConfig, | ||
): Promise<GetImageResult> { | ||
if (!options || typeof options !== 'object') { | ||
throw new AstroError({ | ||
|
@@ -65,6 +75,10 @@ export async function getImage( | |
src: await resolveSrc(options.src), | ||
}; | ||
|
||
let originalWidth: number | undefined; | ||
let originalHeight: number | undefined; | ||
let originalFormat: string | undefined; | ||
|
||
// Infer size for remote images if inferSize is true | ||
if ( | ||
options.inferSize && | ||
|
@@ -74,6 +88,9 @@ export async function getImage( | |
const result = await inferRemoteSize(resolvedOptions.src); // Directly probe the image URL | ||
resolvedOptions.width ??= result.width; | ||
resolvedOptions.height ??= result.height; | ||
originalWidth = result.width; | ||
originalHeight = result.height; | ||
originalFormat = result.format; | ||
delete resolvedOptions.inferSize; // Delete so it doesn't end up in the attributes | ||
} | ||
|
||
|
@@ -88,8 +105,49 @@ export async function getImage( | |
(resolvedOptions.src.clone ?? resolvedOptions.src) | ||
: resolvedOptions.src; | ||
|
||
if (isESMImportedImage(clonedSrc)) { | ||
originalWidth = clonedSrc.width; | ||
originalHeight = clonedSrc.height; | ||
originalFormat = clonedSrc.format; | ||
} | ||
|
||
if (originalWidth && originalHeight) { | ||
// Calculate any missing dimensions from the aspect ratio, if available | ||
const aspectRatio = originalWidth / originalHeight; | ||
if (resolvedOptions.height && !resolvedOptions.width) { | ||
resolvedOptions.width = Math.round(resolvedOptions.height * aspectRatio); | ||
} else if (resolvedOptions.width && !resolvedOptions.height) { | ||
resolvedOptions.height = Math.round(resolvedOptions.width / aspectRatio); | ||
} else if (!resolvedOptions.width && !resolvedOptions.height) { | ||
resolvedOptions.width = originalWidth; | ||
resolvedOptions.height = originalHeight; | ||
} | ||
} | ||
resolvedOptions.src = clonedSrc; | ||
|
||
const layout = options.layout ?? imageConfig.experimentalLayout; | ||
|
||
if (imageConfig.experimentalResponsiveImages && layout) { | ||
resolvedOptions.widths ||= getWidths({ | ||
width: resolvedOptions.width, | ||
layout, | ||
originalWidth, | ||
breakpoints: isLocalService(service) ? LIMITED_RESOLUTIONS : DEFAULT_RESOLUTIONS, | ||
}); | ||
resolvedOptions.sizes ||= getSizesAttribute({ width: resolvedOptions.width, layout }); | ||
|
||
if (resolvedOptions.priority) { | ||
resolvedOptions.loading ??= 'eager'; | ||
resolvedOptions.decoding ??= 'sync'; | ||
resolvedOptions.fetchpriority ??= 'high'; | ||
} else { | ||
resolvedOptions.loading ??= 'lazy'; | ||
resolvedOptions.decoding ??= 'async'; | ||
resolvedOptions.fetchpriority ??= 'auto'; | ||
} | ||
delete resolvedOptions.priority; | ||
} | ||
|
||
const validatedOptions = service.validateOptions | ||
? await service.validateOptions(resolvedOptions, imageConfig) | ||
: resolvedOptions; | ||
|
@@ -100,13 +158,23 @@ export async function getImage( | |
: []; | ||
|
||
let imageURL = await service.getURL(validatedOptions, imageConfig); | ||
|
||
const matchesOriginal = (transform: ImageTransform) => | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In theory, couldn't the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think in this scenario it can't, as this is just checking if it's the srcset entry which is the same as the src. I don't think we expose a way to set this separately. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can't an image service set a different quality for a srcset transform? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess in theory. I'm comfortable with not supporting that though. It's a terrible idea. |
||
transform.width === originalWidth && | ||
transform.height === originalHeight && | ||
transform.format === originalFormat; | ||
|
||
let srcSets: SrcSetValue[] = await Promise.all( | ||
srcSetTransforms.map(async (srcSet) => ({ | ||
transform: srcSet.transform, | ||
url: await service.getURL(srcSet.transform, imageConfig), | ||
descriptor: srcSet.descriptor, | ||
attributes: srcSet.attributes, | ||
})), | ||
srcSetTransforms.map(async (srcSet) => { | ||
return { | ||
transform: srcSet.transform, | ||
url: matchesOriginal(srcSet.transform) | ||
? imageURL | ||
: await service.getURL(srcSet.transform, imageConfig), | ||
descriptor: srcSet.descriptor, | ||
attributes: srcSet.attributes, | ||
}; | ||
}), | ||
); | ||
|
||
if ( | ||
|
@@ -120,12 +188,16 @@ export async function getImage( | |
propsToHash, | ||
originalFilePath, | ||
); | ||
srcSets = srcSetTransforms.map((srcSet) => ({ | ||
transform: srcSet.transform, | ||
url: globalThis.astroAsset.addStaticImage!(srcSet.transform, propsToHash, originalFilePath), | ||
descriptor: srcSet.descriptor, | ||
attributes: srcSet.attributes, | ||
})); | ||
srcSets = srcSetTransforms.map((srcSet) => { | ||
return { | ||
transform: srcSet.transform, | ||
url: matchesOriginal(srcSet.transform) | ||
? imageURL | ||
: globalThis.astroAsset.addStaticImage!(srcSet.transform, propsToHash, originalFilePath), | ||
descriptor: srcSet.descriptor, | ||
attributes: srcSet.attributes, | ||
}; | ||
}); | ||
} | ||
|
||
return { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should people / image service be able to customise this? I'm notably talking about Vercel here, where the breakpoints needs to somewhat matches the allowed widths (hahaha)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did consider adding a
breakpoints
option to the config, though people can still setwidths
manually. Maybe it would be a good idea.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, config now supports
image.experimentalBreakpoints