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

Add inferSize to getImage so width and height are optional for remote images #9976

Merged
merged 42 commits into from
Feb 15, 2024
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
58e354a
add remote image dimension probing, and param for getImage options, a…
OliverSpeir Jan 30, 2024
a5c2f04
add inferSize parameter to getImage, which probes remote image size, …
OliverSpeir Feb 4, 2024
a39438b
add fixture and tests
OliverSpeir Feb 4, 2024
71fbdca
Merge branch 'main' of https://github.com/withastro/astro into get-re…
OliverSpeir Feb 5, 2024
466c033
add changeset
OliverSpeir Feb 5, 2024
eae056a
attempt to fix pnpm-lock.yaml
OliverSpeir Feb 5, 2024
c270d3c
fix pnpm-lock.yaml again
OliverSpeir Feb 5, 2024
4847cd1
pnpm-lock spacing
OliverSpeir Feb 5, 2024
579cb87
fix pnpm-lock AGAIN
OliverSpeir Feb 5, 2024
b078878
Merge branch 'main' into get-remote-dimensions
florian-lefebvre Feb 5, 2024
10507bc
better description of error for docgen
OliverSpeir Feb 6, 2024
28cdb58
improve failed to probe error message and correct required dimensions…
OliverSpeir Feb 6, 2024
551c7f1
increase timeout for mdx tests
OliverSpeir Feb 6, 2024
488858c
increasing mdx timeout to 2min to see if it passes, will reduce if it…
OliverSpeir Feb 6, 2024
f408f33
setting mdx timeout to 70 seconds
OliverSpeir Feb 6, 2024
374293d
Update packages/astro/src/assets/services/service.ts
OliverSpeir Feb 8, 2024
ab9cd45
refactor to move logic to getImage instead of validateOptions and to…
OliverSpeir Feb 8, 2024
5bac12d
fix broken link created by docs PR
OliverSpeir Feb 8, 2024
603c4df
remove the probe-image-size library due to its use of node APIs
OliverSpeir Feb 10, 2024
b642cb3
undo all changes to service.ts that were left after moving inferSize…
OliverSpeir Feb 12, 2024
878e9b6
Merge branch 'main' into get-remote-dimensions
OliverSpeir Feb 12, 2024
457ada2
update error message
sarah11918 Feb 13, 2024
624c86a
remove probe-image-size library all together, update error message, a…
OliverSpeir Feb 13, 2024
50b91c4
Merge branch 'get-remote-dimensions' of https://github.com/OliverSpei…
OliverSpeir Feb 13, 2024
3f09a2c
Update .changeset/tame-cameras-change.md
OliverSpeir Feb 13, 2024
a9fc180
Update .changeset/tame-cameras-change.md
OliverSpeir Feb 13, 2024
75422d0
reword the error message based on Sarah's previous review
OliverSpeir Feb 13, 2024
2a2050a
Merge branch 'get-remote-dimensions' of https://github.com/OliverSpei…
OliverSpeir Feb 13, 2024
39bc8d5
Merge branch 'main' into get-remote-dimensions
OliverSpeir Feb 13, 2024
a388af1
remove probe-image-size from pnpmlock
OliverSpeir Feb 13, 2024
63cecd5
fix lockfile?
OliverSpeir Feb 13, 2024
6d0d868
update error message name
OliverSpeir Feb 13, 2024
47ce1bf
move image-size into vendor folder
OliverSpeir Feb 13, 2024
b845055
add eslint ignore to a line in image-size
OliverSpeir Feb 13, 2024
f10c5a4
test if change to mdx test timeout was needed
OliverSpeir Feb 14, 2024
5dee749
Update .changeset/tame-cameras-change.md
ematipico Feb 14, 2024
ea220a0
update changset syntax
OliverSpeir Feb 14, 2024
58f0dc5
Merge branch 'main' into get-remote-dimensions
OliverSpeir Feb 14, 2024
f4a71e6
Merge branch 'main' into get-remote-dimensions
ematipico Feb 14, 2024
fdf64de
patch parse heif to account for filetype block being out of order
OliverSpeir Feb 14, 2024
60ee177
Merge branch 'get-remote-dimensions' of https://github.com/OliverSpei…
OliverSpeir Feb 14, 2024
f1fbb2f
Merge branch 'main' into get-remote-dimensions
OliverSpeir Feb 14, 2024
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
17 changes: 17 additions & 0 deletions .changeset/tame-cameras-change.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
"astro": minor
---

Remote images can now have their dimensions inferred just like local images, allow width and height to be optional when inferSize is set to true.
OliverSpeir marked this conversation as resolved.
Show resolved Hide resolved

```astro
---
import { Image, Picture, getImage } from 'astro:assets';
const myPic = await getImage({src: "https://example.com/example.png", inferSize: true, alt: ""})
---
<Image src="https://example.com/example.png" inferSize={true} alt="">
<Picture src="https://example.com/example.png" inferSize={true} alt="">
ematipico marked this conversation as resolved.
Show resolved Hide resolved
```


> added to trigger tests
OliverSpeir marked this conversation as resolved.
Show resolved Hide resolved
3 changes: 3 additions & 0 deletions packages/astro/client.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ declare module 'astro:assets' {
export type RemoteImageProps = import('./dist/type-utils.js').Simplify<
import('./dist/assets/types.js').RemoteImageProps<ImgAttributes>
>;
export type RemoteImagePropsInferSize = import('./dist/type-utils.js').Simplify<
import('./dist/assets/types.js').RemoteImagePropsInferSize<ImgAttributes>
>;
export const { getImage, getConfiguredImageService, imageConfig, Image, Picture }: AstroAssets;
}

Expand Down
9 changes: 7 additions & 2 deletions packages/astro/components/Image.astro
Original file line number Diff line number Diff line change
@@ -1,12 +1,17 @@
---
import { getImage, type LocalImageProps, type RemoteImageProps } from 'astro:assets';
import {
getImage,
type LocalImageProps,
type RemoteImageProps,
type RemoteImagePropsInferSize,
} from 'astro:assets';
import { AstroError, AstroErrorData } from '../dist/core/errors/index.js';
import type { HTMLAttributes } from '../types';

// The TypeScript diagnostic for JSX props uses the last member of the union to suggest props, so it would be better for
// LocalImageProps to be last. Unfortunately, when we do this the error messages that remote images get are complete nonsense
// Not 100% sure how to fix this, seems to be a TypeScript issue. Unfortunate.
type Props = LocalImageProps | RemoteImageProps;
type Props = LocalImageProps | RemoteImagePropsInferSize | RemoteImageProps;

const props = Astro.props;

Expand Down
9 changes: 7 additions & 2 deletions packages/astro/components/Picture.astro
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
---
import { getImage, type LocalImageProps, type RemoteImageProps } from 'astro:assets';
import {
getImage,
type LocalImageProps,
type RemoteImageProps,
type RemoteImagePropsInferSize,
} from 'astro:assets';
import type { GetImageResult, ImageOutputFormat } from '../dist/@types/astro';
import { isESMImportedImage } from '../dist/assets/utils/imageKind';
import { AstroError, AstroErrorData } from '../dist/core/errors/index.js';
import type { HTMLAttributes } from '../types';

type Props = (LocalImageProps | RemoteImageProps) & {
type Props = (LocalImageProps | RemoteImagePropsInferSize | RemoteImageProps) & {
formats?: ImageOutputFormat[];
fallbackFormat?: ImageOutputFormat;
pictureAttributes?: HTMLAttributes<'picture'>;
Expand Down
161 changes: 81 additions & 80 deletions packages/astro/src/assets/services/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { DEFAULT_HASH_PROPS, DEFAULT_OUTPUT_FORMAT, VALID_SUPPORTED_FORMATS } fr
import type { ImageOutputFormat, ImageTransform, UnresolvedSrcSetValue } from '../types.js';
import { isESMImportedImage } from '../utils/imageKind.js';
import { isRemoteAllowed } from '../utils/remotePattern.js';
import probe from 'probe-image-size';

export type ImageService = LocalImageService | ExternalImageService;

Expand Down Expand Up @@ -140,88 +141,88 @@ export type BaseServiceTransform = {
*/
export const baseService: Omit<LocalImageService, 'transform'> = {
propertiesToHash: DEFAULT_HASH_PROPS,
validateOptions(options) {
// `src` is missing or is `undefined`.
if (!options.src || (typeof options.src !== 'string' && typeof options.src !== 'object')) {
throw new AstroError({
...AstroErrorData.ExpectedImage,
message: AstroErrorData.ExpectedImage.message(
JSON.stringify(options.src),
typeof options.src,
JSON.stringify(options, (_, v) => (v === undefined ? null : v))
),
});
}

if (!isESMImportedImage(options.src)) {
// User passed an `/@fs/` path or a filesystem path instead of the full image.
if (
options.src.startsWith('/@fs/') ||
(!isRemotePath(options.src) && !options.src.startsWith('/'))
) {
throw new AstroError({
...AstroErrorData.LocalImageUsedWrongly,
message: AstroErrorData.LocalImageUsedWrongly.message(options.src),
});
}

// For remote images, width and height are explicitly required as we can't infer them from the file
let missingDimension: 'width' | 'height' | 'both' | undefined;
if (!options.width && !options.height) {
missingDimension = 'both';
} else if (!options.width && options.height) {
missingDimension = 'width';
} else if (options.width && !options.height) {
missingDimension = 'height';
}

if (missingDimension) {
throw new AstroError({
...AstroErrorData.MissingImageDimension,
message: AstroErrorData.MissingImageDimension.message(missingDimension, options.src),
});
}
} else {
if (!VALID_SUPPORTED_FORMATS.includes(options.src.format as any)) {
throw new AstroError({
...AstroErrorData.UnsupportedImageFormat,
message: AstroErrorData.UnsupportedImageFormat.message(
options.src.format,
options.src.src,
VALID_SUPPORTED_FORMATS
),
});
}

if (options.widths && options.densities) {
throw new AstroError(AstroErrorData.IncompatibleDescriptorOptions);
}

// We currently do not support processing SVGs, so whenever the input format is a SVG, force the output to also be one
if (options.src.format === 'svg') {
options.format = 'svg';
}

if (
(options.src.format === 'svg' && options.format !== 'svg') ||
(options.src.format !== 'svg' && options.format === 'svg')
) {
throw new AstroError(AstroErrorData.UnsupportedImageConversion);
async validateOptions(options) {
// `src` is missing or is `undefined`.
if (!options.src || (typeof options.src !== 'string' && typeof options.src !== 'object')) {
throw new AstroError({
...AstroErrorData.ExpectedImage,
message: AstroErrorData.ExpectedImage.message(
JSON.stringify(options.src),
typeof options.src,
JSON.stringify(options)
),
});
}

// Infer size for remote images if inferSize is true
if (options.inferSize && !isESMImportedImage(options.src)) {
try {
const result = await probe(options.src); // Directly probe the image URL
OliverSpeir marked this conversation as resolved.
Show resolved Hide resolved
options.width = options.width || result.width;
options.height = options.height || result.height;
OliverSpeir marked this conversation as resolved.
Show resolved Hide resolved
} catch {
throw new AstroError({
...AstroErrorData.FailedToProbeRemoteImage,
message: AstroErrorData.FailedToProbeRemoteImage.message(options.src),
});
}
}
}

// If the user didn't specify a format, we'll default to `webp`. It offers the best ratio of compatibility / quality
// In the future, hopefully we can replace this with `avif`, alas, Edge. See https://caniuse.com/avif
if (!options.format) {
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;
},
// Handle local and ESM-imported images
if (!isESMImportedImage(options.src)) {
if (options.src.startsWith('/@fs/') || (!isRemotePath(options.src) && !options.src.startsWith('/'))) {
throw new AstroError({
...AstroErrorData.LocalImageUsedWrongly,
message: AstroErrorData.LocalImageUsedWrongly.message(options.src),
});
}

if (!options.width || !options.height) {
const missingDimension = !options.width && !options.height ? 'both' :
!options.width ? 'width' : 'height';
throw new AstroError({
...AstroErrorData.MissingImageDimension,
message: AstroErrorData.MissingImageDimension.message(missingDimension, options.src),
});
}
} else {
if (!VALID_SUPPORTED_FORMATS.includes(options.src.format)) {
throw new AstroError({
...AstroErrorData.UnsupportedImageFormat,
message: AstroErrorData.UnsupportedImageFormat.message(
options.src.format,
options.src.src,
VALID_SUPPORTED_FORMATS
),
});
}

if (options.widths && options.densities) {
throw new AstroError(AstroErrorData.IncompatibleDescriptorOptions);
}

if (options.src.format === 'svg') {
options.format = 'svg';
}

if ((options.src.format === 'svg' && options.format !== 'svg') ||
(options.src.format !== 'svg' && options.format === 'svg')) {
throw new AstroError(AstroErrorData.UnsupportedImageConversion);
}
}

// Default to 'webp' format if none is specified
options.format = options.format || DEFAULT_OUTPUT_FORMAT;

// Round width and height to avoid floating point numbers
if (options.width) options.width = Math.round(options.width);
if (options.height) options.height = Math.round(options.height);

// Delete inferSize so it doesn't become an attribute
if (options.inferSize) delete options.inferSize;

return options;
},
getHTMLAttributes(options) {
const { targetWidth, targetHeight } = getTargetDimensions(options);
const { src, width, height, format, quality, densities, widths, formats, ...attributes } =
Expand Down
25 changes: 25 additions & 0 deletions packages/astro/src/assets/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ export type SrcSetValue = UnresolvedSrcSetValue & {
*/
export type UnresolvedImageTransform = Omit<ImageTransform, 'src'> & {
src: ImageMetadata | string | Promise<{ default: ImageMetadata }>;
inferSize?: boolean;
};

/**
Expand Down Expand Up @@ -188,4 +189,28 @@ export type RemoteImageProps<T> = WithRequired<ImageSharedProps<T>, 'width' | 'h
* ```
*/
src: string;
/**
* Option to allow remote images to have their dimensions inferred from the original. Behavior is behind an option due to potential performance consequences in SSR pages.
* In this type inferSize is false or undefined so width and height are required
*/
inferSize?: false | undefined;
};

export type RemoteImagePropsInferSize<T> = ImageSharedProps<T> & {
/**
* URL of a remote image. Can start with a protocol (ex: `https://`) or alternatively `/`, or `Astro.url`, for images in the `public` folder
*
* Remote images are not optimized, and require both `width` and `height` to be set.
*
* **Example**:
* ```
* <Image src="https://example.com/image.png" width={450} height={300} alt="..." />
* ```
*/
src: string;
/**
* Option to allow remote images to have their dimensions inferred from the original. Behavior is behind an option due to potential performance consequences in SSR pages.
* In this type inferSize must be true, so that width and height don't need to be required
*/
inferSize: true;
};
18 changes: 16 additions & 2 deletions packages/astro/src/core/errors/errors-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -530,8 +530,22 @@ export const MissingImageDimension = {
message: (missingDimension: 'width' | 'height' | 'both', imageURL: string) =>
`Missing ${
missingDimension === 'both' ? 'width and height attributes' : `${missingDimension} attribute`
} for ${imageURL}. When using remote images, both dimensions are always required in order to avoid CLS.`,
hint: 'If your image is inside your `src` folder, you probably meant to import it instead. See [the Imports guide for more information](https://docs.astro.build/en/guides/imports/#other-assets).',
} for ${imageURL}. When using remote images, both dimensions are required unless in order to avoid CLS.`,
hint: 'If your image is inside your `src` folder, you probably meant to import it instead. See [the Imports guide for more information](https://docs.astro.build/en/guides/imports/#other-assets). You can also use `inferSize={true}` for remote images to get the original dimensions.',
} satisfies ErrorData;
/**
* @docs
* @message
* Failed to get the dimensions for `IMAGE_URL`.
* @description
* Probing the remote image's dimensions failed, this is typically caused by an incorrect URL or attempting to infer the size of an image in the public folder. Infering the size of images in public folder is not possible. Probing is done by the [probe-image-size](https://www.npmjs.com/package/probe-image-size) package.
*/
export const FailedToProbeRemoteImage = {
name: 'FailedToProbeRemoteImage',
OliverSpeir marked this conversation as resolved.
Show resolved Hide resolved
title: 'Failed to probe remote image dimensions',
OliverSpeir marked this conversation as resolved.
Show resolved Hide resolved
message: (imageURL: string) =>
`Failed to get the dimensions for ${imageURL}.`,
hint: 'Verify your image URL is accurate. If you are using an image in public folder, you will not be able to infer its size.',
} satisfies ErrorData;
/**
* @docs
Expand Down
71 changes: 71 additions & 0 deletions packages/astro/test/core-image-infersize.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import { expect } from 'chai';
import * as cheerio from 'cheerio';
import { Writable } from 'node:stream';

import { Logger } from '../dist/core/logger/core.js';
import { loadFixture } from './test-utils.js';

describe('astro:image:infersize', () => {
/** @type {import('./test-utils').Fixture} */
let fixture;

describe('dev', () => {
/** @type {import('./test-utils').DevServer} */
let devServer;
/** @type {Array<{ type: any, level: 'error', message: string; }>} */
let logs = [];

before(async () => {
fixture = await loadFixture({
root: './fixtures/core-image-infersize/',
});

devServer = await fixture.startDevServer({
logger: new Logger({
level: 'error',
dest: new Writable({
objectMode: true,
write(event, _, callback) {
logs.push(event);
callback();
},
}),
}),
});
});

after(async () => {
await devServer.stop();
});

describe('inferSize works', () => {
let $;
before(async () => {
let res = await fixture.fetch('/');
let html = await res.text();
$ = cheerio.load(html);
});

it('Image component works', async () => {
let $img = $('img');
expect(
$img.attr('src').startsWith('/_image') && $img.attr('src').endsWith('f=webp')
).to.equal(true);
});

it('Picture component works', async () => {
let $img = $('picture img');
expect(
$img.attr('src').startsWith('/_image') && $img.attr('src').endsWith('f=png')
).to.equal(true);
});

it('getImage works', async () => {
let $img = $('#getImage');
expect(
$img.attr('src').startsWith('/_image') && $img.attr('src').endsWith('f=webp')
).to.equal(true);
});
});
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { defineConfig } from 'astro/config';

// https://astro.build/config
export default defineConfig({
image: {
domains: ['avatars.githubusercontent.com'],
},
});
11 changes: 11 additions & 0 deletions packages/astro/test/fixtures/core-image-infersize/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"name": "@test/core-image-remark-infersize",
"version": "0.0.0",
"private": true,
"dependencies": {
"astro": "workspace:*"
},
"scripts": {
"dev": "astro dev"
}
}
Loading
Loading