Skip to content
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: add crop support to image services #12414

Merged
merged 7 commits into from
Nov 12, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 22 additions & 16 deletions packages/astro/components/Image.astro
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,20 @@ if (typeof props.height === 'string') {
props.height = parseInt(props.height);
}

const { experimentalResponsiveImages } = imageConfig;

const layoutClassMap = {
fixed: 'aim-fi',
responsive: 'aim-re',
};

if (experimentalResponsiveImages) {
// Apply defaults from imageConfig if not provided
props.layout ??= imageConfig.experimentalLayout;
props.fit ??= imageConfig.experimentalObjectFit ?? 'cover';
props.position ??= imageConfig.experimentalObjectPosition ?? 'center';
}

const image = await getImage(props);

const additionalAttributes: HTMLAttributes<'img'> = {};
Expand All @@ -34,24 +48,15 @@ if (import.meta.env.DEV) {
additionalAttributes['data-image-component'] = 'true';
}

const { experimentalResponsiveImages } = imageConfig;

const layoutClassMap = {
fixed: 'aim-fi',
responsive: 'aim-re',
};

const cssFitValues = ['fill', 'contain', 'cover', 'scale-down'];
const objectFit = props.fit ?? imageConfig.experimentalObjectFit ?? 'cover';
const objectPosition = props.position ?? imageConfig.experimentalObjectPosition ?? 'center';

// The style prop can't be spread when using define:vars, so we need to extract it here
// @see https://github.com/withastro/compiler/issues/1050
const { style = '', class: className, ...attrs } = { ...additionalAttributes, ...image.attributes };
---

{
experimentalResponsiveImages ? (
experimentalResponsiveImages && props.layout ? (
<img
src={image.src}
{...attrs}
Expand All @@ -64,12 +69,13 @@ const { style = '', class: className, ...attrs } = { ...additionalAttributes, ..
}

<style
define:vars={experimentalResponsiveImages && {
w: image.attributes.width ?? props.width ?? image.options.width,
h: image.attributes.height ?? props.height ?? image.options.height,
fit: cssFitValues.includes(objectFit) && objectFit,
pos: objectPosition,
}}
define:vars={experimentalResponsiveImages &&
props.layout && {
w: image.attributes.width ?? props.width ?? image.options.width,
h: image.attributes.height ?? props.height ?? image.options.height,
fit: cssFitValues.includes(props.fit) && props.fit,
pos: props.position,
}}
>
/* Shared by all Astro images */
.aim {
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/assets/consts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,4 @@ export const VALID_SUPPORTED_FORMATS = [
] as const;
export const DEFAULT_OUTPUT_FORMAT = 'webp' as const;
export const VALID_OUTPUT_FORMATS = ['avif', 'png', 'webp', 'jpeg', 'jpg', 'svg'] as const;
export const DEFAULT_HASH_PROPS = ['src', 'width', 'height', 'format', 'quality'];
export const DEFAULT_HASH_PROPS = ['src', 'width', 'height', 'format', 'quality', 'fit', 'position'];
6 changes: 3 additions & 3 deletions packages/astro/src/assets/layout.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { ImageLayout } from '../types/public/index.js';
import type { ImageLayout } from './types.js';

// Common screen widths. These will be filtered according to the image size and layout
export const DEFAULT_RESOLUTIONS = [
Expand Down Expand Up @@ -33,9 +33,9 @@ export const LIMITED_RESOLUTIONS = [

/**
* Gets the breakpoints for an image, based on the layout and width
*
*
* The rules are as follows:
*
*
* - For full-width layout we return all breakpoints smaller than the original image width
* - For fixed layout we return 1x and 2x the requested width, unless the original image is smaller than that.
* - For responsive layout we return all breakpoints smaller than 2x the requested width, unless the original image is smaller than that.
Expand Down
23 changes: 21 additions & 2 deletions packages/astro/src/assets/services/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@ import { AstroError, AstroErrorData } from '../../core/errors/index.js';
import { isRemotePath, joinPaths } from '../../core/path.js';
import type { AstroConfig } from '../../types/public/config.js';
import { DEFAULT_HASH_PROPS, DEFAULT_OUTPUT_FORMAT, VALID_SUPPORTED_FORMATS } from '../consts.js';
import type { ImageOutputFormat, ImageTransform, UnresolvedSrcSetValue } from '../types.js';
import type {
ImageFit,
ImageOutputFormat,
ImageTransform,
UnresolvedSrcSetValue,
} from '../types.js';
import { isESMImportedImage } from '../utils/imageKind.js';
import { isRemoteAllowed } from '../utils/remotePattern.js';

Expand Down Expand Up @@ -116,6 +121,8 @@ export type BaseServiceTransform = {
height?: number;
format: string;
quality?: string | null;
fit?: ImageFit;
position?: string;
};

const sortNumeric = (a: number, b: number) => a - b;
Expand Down Expand Up @@ -221,7 +228,13 @@ export const baseService: Omit<LocalImageService, 'transform'> = {
// 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);

if (options.layout && options.width && options.height) {
options.fit ??= 'cover';
delete options.layout;
}
if (options.fit === 'none') {
delete options.fit;
}
return options;
},
getHTMLAttributes(options) {
Expand All @@ -237,6 +250,8 @@ export const baseService: Omit<LocalImageService, 'transform'> = {
formats,
layout,
priority,
fit,
position,
...attributes
} = options;
return {
Expand Down Expand Up @@ -344,6 +359,8 @@ export const baseService: Omit<LocalImageService, 'transform'> = {
h: 'height',
q: 'quality',
f: 'format',
fit: 'fit',
position: 'position',
};

Object.entries(params).forEach(([param, key]) => {
Expand All @@ -366,6 +383,8 @@ export const baseService: Omit<LocalImageService, 'transform'> = {
height: params.has('h') ? parseInt(params.get('h')!) : undefined,
format: params.get('f') as ImageOutputFormat,
quality: params.get('q'),
fit: params.get('fit') as ImageFit,
position: params.get('position') ?? undefined,
};

return transform;
Expand Down
42 changes: 35 additions & 7 deletions packages/astro/src/assets/services/sharp.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { FormatEnum, SharpOptions } from 'sharp';
import type { FitEnum, FormatEnum, SharpOptions } from 'sharp';
import { AstroError, AstroErrorData } from '../../core/errors/index.js';
import type { ImageOutputFormat, ImageQualityPreset } from '../types.js';
import type { ImageFit, ImageOutputFormat, ImageQualityPreset } from '../types.js';
import {
type BaseServiceTransform,
type LocalImageService,
Expand Down Expand Up @@ -38,6 +38,16 @@ async function loadSharp() {
return sharpImport;
}

const fitMap: Record<ImageFit, keyof FitEnum> = {
fill: 'fill',
contain: 'inside',
cover: 'cover',
none: 'outside',
'scale-down': 'inside',
'outside': 'outside',
'inside': 'inside',
};

const sharpService: LocalImageService<SharpImageServiceConfig> = {
validateOptions: baseService.validateOptions,
getURL: baseService.getURL,
Expand All @@ -46,7 +56,6 @@ const sharpService: LocalImageService<SharpImageServiceConfig> = {
getSrcSet: baseService.getSrcSet,
async transform(inputBuffer, transformOptions, config) {
if (!sharp) sharp = await loadSharp();

const transform: BaseServiceTransform = transformOptions as BaseServiceTransform;

// Return SVGs as-is
Expand All @@ -62,11 +71,30 @@ const sharpService: LocalImageService<SharpImageServiceConfig> = {
// always call rotate to adjust for EXIF data orientation
result.rotate();

// Never resize using both width and height at the same time, prioritizing width.
if (transform.height && !transform.width) {
result.resize({ height: Math.round(transform.height) });
// If `fit` isn't set then use old behavior:
// - Do not use both width and height for resizing, and prioritize width over height
// - Allow enlarging images

const withoutEnlargement = Boolean(transform.fit);
if (transform.width && transform.height && transform.fit) {
const fit: keyof FitEnum = fitMap[transform.fit] ?? 'inside';
result.resize({
width: Math.round(transform.width),
height: Math.round(transform.height),
fit,
position: transform.position,
withoutEnlargement,
});
} else if (transform.height && !transform.width) {
result.resize({
height: Math.round(transform.height),
withoutEnlargement,
});
} else if (transform.width) {
result.resize({ width: Math.round(transform.width) });
result.resize({
width: Math.round(transform.width),
withoutEnlargement,
});
}

if (transform.format) {
Expand Down
7 changes: 5 additions & 2 deletions packages/astro/src/assets/types.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import type { ImageLayout } from '../types/public/index.js';
import type { OmitPreservingIndexSignature, Simplify, WithRequired } from '../type-utils.js';
import type { VALID_INPUT_FORMATS, VALID_OUTPUT_FORMATS } from './consts.js';
import type { ImageService } from './services/service.js';
Expand All @@ -7,6 +6,8 @@ export type ImageQualityPreset = 'low' | 'mid' | 'high' | 'max' | (string & {});
export type ImageQuality = ImageQualityPreset | number;
export type ImageInputFormat = (typeof VALID_INPUT_FORMATS)[number];
export type ImageOutputFormat = (typeof VALID_OUTPUT_FORMATS)[number] | (string & {});
export type ImageLayout = 'responsive' | 'fixed' | 'full-width' | 'none';
export type ImageFit = 'fill' | 'contain' | 'cover' | 'none' | 'scale-down' | (string & {});

export type AssetsGlobalStaticImagesList = Map<
string,
Expand Down Expand Up @@ -87,6 +88,8 @@ export type ImageTransform = {
height?: number | undefined;
quality?: ImageQuality | undefined;
format?: ImageOutputFormat | undefined;
fit?: ImageFit | undefined;
position?: string | undefined;
[key: string]: any;
};

Expand Down Expand Up @@ -157,7 +160,7 @@ type ImageSharedProps<T> = T & {

layout?: ImageLayout;

fit?: 'fill' | 'contain' | 'cover' | 'none' | 'scale-down' | (string & {});
fit?: ImageFit;

position?: string;
} & (
Expand Down
5 changes: 2 additions & 3 deletions packages/astro/src/types/public/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import type { REDIRECT_STATUS_CODES } from '../../core/constants.js';
import type { Logger, LoggerLevel } from '../../core/logger/core.js';
import type { EnvSchema } from '../../env/schema.js';
import type { AstroIntegration } from './integrations.js';
import type { ImageFit, ImageLayout } from '../../assets/types.js';

export type Locales = (string | { codes: string[]; path: string })[];

Expand Down Expand Up @@ -1091,7 +1092,7 @@ export interface ViteUserConfig extends OriginalViteUserConfig {
* The default object-fit value for responsive images. Can be overridden by the `fit` prop on the image component.
* Requires the `experimental.responsiveImages` flag to be enabled.
*/
experimentalObjectFit?: 'contain' | 'cover' | 'fill' | 'none' | 'scale-down' | (string & {});
experimentalObjectFit?: ImageFit;
/**
* @docs
* @name image.experimentalObjectPosition
Expand Down Expand Up @@ -1766,8 +1767,6 @@ export interface ViteUserConfig extends OriginalViteUserConfig {
};
}

export type ImageLayout = 'responsive' | 'fixed' | 'full-width' | 'none';

/**
* Resolved Astro Config
*
Expand Down
56 changes: 55 additions & 1 deletion packages/astro/test/core-image-layout.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
/** @type {import('./test-utils').DevServer} */
let devServer;
/** @type {Array<{ type: any, level: 'error', message: string; }>} */
let logs = [];

Check notice on line 19 in packages/astro/test/core-image-layout.test.js

View workflow job for this annotation

GitHub Actions / Lint

lint/correctness/noUnusedVariables

This variable is unused.

before(async () => {
fixture = await loadFixture({
Expand Down Expand Up @@ -134,7 +134,7 @@
assert.match(style, /\.aim\[/);
assert.match(style, /\.aim-re\[/);
assert.match(style, /\.aim-fi\[/);
})
});
});

describe('srcsets', () => {
Expand Down Expand Up @@ -180,6 +180,60 @@
});
});

describe('generated URLs', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Github annotation says that logs (line 19) is unused

let $;
before(async () => {
let res = await fixture.fetch('/fit');
let html = await res.text();
$ = cheerio.load(html);
});
it('generates width and height in image URLs when both are provided', () => {
let $img = $('#local-both img');
const aspectRatio = 300 / 400;
const srcset = parseSrcset($img.attr('srcset'));
for (const { url } of srcset) {
const params = new URL(url, 'https://example.com').searchParams;
const width = parseInt(params.get('w'));
const height = parseInt(params.get('h'));
assert.equal(width / height, aspectRatio);
}
});

it('does not pass through fit and position', async () => {
const fit = $('#fit-cover img');
assert.ok(!fit.attr('fit'));
const position = $('#position img');
assert.ok(!position.attr('position'));
});

it('sets a default fit of "cover" when no fit is provided', () => {
let $img = $('#fit-default img');
const srcset = parseSrcset($img.attr('srcset'));
for (const { url } of srcset) {
const params = new URL(url, 'https://example.com').searchParams;
assert.equal(params.get('fit'), 'cover');
}
});

it('sets a fit of "contain" when fit="contain" is provided', () => {
let $img = $('#fit-contain img');
const srcset = parseSrcset($img.attr('srcset'));
for (const { url } of srcset) {
const params = new URL(url, 'https://example.com').searchParams;
assert.equal(params.get('fit'), 'contain');
}
});

it('sets no fit when fit="none" is provided', () => {
let $img = $('#fit-none img');
const srcset = parseSrcset($img.attr('srcset'));
for (const { url } of srcset) {
const params = new URL(url, 'https://example.com').searchParams;
assert.ok(!params.has('fit'));
}
});
});

describe('remote images', () => {
describe('srcset', () => {
let $;
Expand Down
Loading
Loading