From b2ae9ee0c42b11ffc1d3f070d1d5ac881aef84ed Mon Sep 17 00:00:00 2001 From: Erika <3019731+Princesseuh@users.noreply.github.com> Date: Wed, 11 Oct 2023 17:53:58 +0200 Subject: [PATCH 1/3] feat(assets): Add support for `srcset` and a Picture component (#8620) Co-authored-by: Sarah Rainsberger --- .changeset/smooth-goats-agree.md | 56 +++++++ packages/astro/client.d.ts | 12 +- packages/astro/components/Image.astro | 8 +- packages/astro/components/Picture.astro | 57 +++++++ packages/astro/package.json | 1 + packages/astro/src/assets/consts.ts | 1 + packages/astro/src/assets/internal.ts | 24 ++- packages/astro/src/assets/services/service.ts | 153 +++++++++++++++--- packages/astro/src/assets/services/sharp.ts | 5 +- packages/astro/src/assets/services/squoosh.ts | 5 +- packages/astro/src/assets/types.ts | 35 +++- .../astro/src/assets/utils/transformToPath.ts | 4 +- .../astro/src/assets/vite-plugin-assets.ts | 1 + packages/astro/src/core/errors/errors-data.ts | 15 ++ packages/astro/test/core-image.test.js | 51 ++++++ .../src/pages/picturecomponent.astro | 6 + .../src/pages/picturecomponent.astro | 12 ++ pnpm-lock.yaml | 7 + 18 files changed, 413 insertions(+), 40 deletions(-) create mode 100644 .changeset/smooth-goats-agree.md create mode 100644 packages/astro/components/Picture.astro create mode 100644 packages/astro/test/fixtures/core-image-ssg/src/pages/picturecomponent.astro create mode 100644 packages/astro/test/fixtures/core-image/src/pages/picturecomponent.astro diff --git a/.changeset/smooth-goats-agree.md b/.changeset/smooth-goats-agree.md new file mode 100644 index 000000000000..3c72235c3404 --- /dev/null +++ b/.changeset/smooth-goats-agree.md @@ -0,0 +1,56 @@ +--- +'astro': minor +--- + +Adds experimental support for generating `srcset` attributes and a new `` component. + +## `srcset` support + +Two new properties have been added to `Image` and `getImage()`: `densities` and `widths`. + +These properties can be used to generate a `srcset` attribute, either based on absolute widths in pixels (e.g. [300, 600, 900]) or pixel density descriptors (e.g. `["2x"]` or `[1.5, 2]`). + + +```astro +--- +import { Image } from "astro"; +import myImage from "./my-image.jpg"; +--- + +My cool image +``` + +```html +My cool image +``` + +## Picture component + +The experimental `` component can be used to generate a `` element with multiple `` elements. + +The example below uses the `format` property to generate a `` in each of the specified image formats: + +```astro +--- +import { Picture } from "astro:assets"; +import myImage from "./my-image.jpg"; +--- + + +``` + +The above code will generate the following HTML, and allow the browser to determine the best image to display: + +```html + + + + My super image in multiple formats! + +``` + +The `Picture` component takes all the same props as the `Image` component, including the new `densities` and `widths` properties. diff --git a/packages/astro/client.d.ts b/packages/astro/client.d.ts index c75ae79712c8..f20c3d089632 100644 --- a/packages/astro/client.d.ts +++ b/packages/astro/client.d.ts @@ -53,6 +53,7 @@ declare module 'astro:assets' { imageConfig: import('./dist/@types/astro.js').AstroConfig['image']; getConfiguredImageService: typeof import('./dist/assets/index.js').getConfiguredImageService; Image: typeof import('./components/Image.astro').default; + Picture: typeof import('./components/Picture.astro').default; }; type ImgAttributes = import('./dist/type-utils.js').WithRequired< @@ -66,17 +67,10 @@ declare module 'astro:assets' { export type RemoteImageProps = import('./dist/type-utils.js').Simplify< import('./dist/assets/types.js').RemoteImageProps >; - export const { getImage, getConfiguredImageService, imageConfig, Image }: AstroAssets; + export const { getImage, getConfiguredImageService, imageConfig, Image, Picture }: AstroAssets; } -type InputFormat = import('./dist/assets/types.js').ImageInputFormat; - -interface ImageMetadata { - src: string; - width: number; - height: number; - format: InputFormat; -} +type ImageMetadata = import('./dist/assets/types.js').ImageMetadata; declare module '*.gif' { const metadata: ImageMetadata; diff --git a/packages/astro/components/Image.astro b/packages/astro/components/Image.astro index 00b0cc5fafbc..a11efd4f9769 100644 --- a/packages/astro/components/Image.astro +++ b/packages/astro/components/Image.astro @@ -23,6 +23,12 @@ if (typeof props.height === 'string') { } const image = await getImage(props); + +const additionalAttributes: Record = {}; + +if (image.srcSet.values.length > 0) { + additionalAttributes.srcset = image.srcSet.attribute; +} --- - + diff --git a/packages/astro/components/Picture.astro b/packages/astro/components/Picture.astro new file mode 100644 index 000000000000..5ea2516d675e --- /dev/null +++ b/packages/astro/components/Picture.astro @@ -0,0 +1,57 @@ +--- +import { getImage, type LocalImageProps, type RemoteImageProps } from 'astro:assets'; +import type { GetImageResult, ImageOutputFormat } from '../dist/@types/astro'; +import { isESMImportedImage } from '../dist/assets/internal'; +import { AstroError, AstroErrorData } from '../dist/core/errors/index.js'; +import type { HTMLAttributes } from '../types'; + +type Props = (LocalImageProps | RemoteImageProps) & { + formats?: ImageOutputFormat[]; + fallbackFormat?: ImageOutputFormat; + pictureAttributes?: HTMLAttributes<'picture'>; +}; + +const { formats = ['webp'], pictureAttributes = {}, ...props } = Astro.props; + +if (props.alt === undefined || props.alt === null) { + throw new AstroError(AstroErrorData.ImageMissingAlt); +} + +const optimizedImages: GetImageResult[] = await Promise.all( + formats.map( + async (format) => + await getImage({ ...props, format: format, widths: props.widths, densities: props.densities }) + ) +); + +const fallbackFormat = + props.fallbackFormat ?? isESMImportedImage(props.src) + ? ['svg', 'gif'].includes(props.src.format) + ? props.src.format + : 'png' + : 'png'; + +const fallbackImage = await getImage({ + ...props, + format: fallbackFormat, + widths: props.widths, + densities: props.densities, +}); + +const additionalAttributes: Record = {}; +if (fallbackImage.srcSet.values.length > 0) { + additionalAttributes.srcset = fallbackImage.srcSet.attribute; +} +--- + + + { + Object.entries(optimizedImages).map(([_, image]) => ( + 0 ? ' , ' + image.srcSet.attribute : ''}`} + type={"image/" + image.options.format} + /> + )) + } + + diff --git a/packages/astro/package.json b/packages/astro/package.json index f64cf1c8a1d0..8c55980e36df 100644 --- a/packages/astro/package.json +++ b/packages/astro/package.json @@ -213,6 +213,7 @@ "mocha": "^10.2.0", "network-information-types": "^0.1.1", "node-mocks-http": "^1.13.0", + "parse-srcset": "^1.0.2", "rehype-autolink-headings": "^6.1.1", "rehype-slug": "^5.0.1", "rehype-toc": "^3.0.2", diff --git a/packages/astro/src/assets/consts.ts b/packages/astro/src/assets/consts.ts index 90dfa599cf21..687582b8ec43 100644 --- a/packages/astro/src/assets/consts.ts +++ b/packages/astro/src/assets/consts.ts @@ -24,4 +24,5 @@ export const VALID_SUPPORTED_FORMATS = [ 'svg', 'avif', ] as const; +export const DEFAULT_OUTPUT_FORMAT = 'webp' as const; export const VALID_OUTPUT_FORMATS = ['avif', 'png', 'webp', 'jpeg', 'jpg', 'svg'] as const; diff --git a/packages/astro/src/assets/internal.ts b/packages/astro/src/assets/internal.ts index 310b746a844e..f7df11f69218 100644 --- a/packages/astro/src/assets/internal.ts +++ b/packages/astro/src/assets/internal.ts @@ -6,6 +6,7 @@ import type { GetImageResult, ImageMetadata, ImageTransform, + SrcSetValue, UnresolvedImageTransform, } from './types.js'; import { matchHostname, matchPattern } from './utils/remotePattern.js'; @@ -93,22 +94,41 @@ export async function getImage( ? await service.validateOptions(resolvedOptions, imageConfig) : resolvedOptions; + // Get all the options for the different srcSets + const srcSetTransforms = service.getSrcSet + ? await service.getSrcSet(validatedOptions, imageConfig) + : []; + let imageURL = await service.getURL(validatedOptions, imageConfig); + let srcSets: SrcSetValue[] = await Promise.all( + srcSetTransforms.map(async (srcSet) => ({ + url: await service.getURL(srcSet.transform, imageConfig), + descriptor: srcSet.descriptor, + attributes: srcSet.attributes, + })) + ); - // In build and for local services, we need to collect the requested parameters so we can generate the final images if ( isLocalService(service) && globalThis.astroAsset.addStaticImage && - // If `getURL` returned the same URL as the user provided, it means the service doesn't need to do anything !(isRemoteImage(validatedOptions.src) && imageURL === validatedOptions.src) ) { imageURL = globalThis.astroAsset.addStaticImage(validatedOptions); + srcSets = srcSetTransforms.map((srcSet) => ({ + url: globalThis.astroAsset.addStaticImage!(srcSet.transform), + descriptor: srcSet.descriptor, + attributes: srcSet.attributes, + })); } return { rawOptions: resolvedOptions, options: validatedOptions, src: imageURL, + srcSet: { + values: srcSets, + attribute: srcSets.map((srcSet) => `${srcSet.url} ${srcSet.descriptor}`).join(', '), + }, attributes: service.getHTMLAttributes !== undefined ? await service.getHTMLAttributes(validatedOptions, imageConfig) diff --git a/packages/astro/src/assets/services/service.ts b/packages/astro/src/assets/services/service.ts index 69894aa0cefc..9812c95c33b6 100644 --- a/packages/astro/src/assets/services/service.ts +++ b/packages/astro/src/assets/services/service.ts @@ -1,7 +1,7 @@ import type { AstroConfig } from '../../@types/astro.js'; import { AstroError, AstroErrorData } from '../../core/errors/index.js'; import { isRemotePath, joinPaths } from '../../core/path.js'; -import { VALID_SUPPORTED_FORMATS } from '../consts.js'; +import { DEFAULT_OUTPUT_FORMAT, VALID_SUPPORTED_FORMATS } from '../consts.js'; import { isESMImportedImage, isRemoteAllowed } from '../internal.js'; import type { ImageOutputFormat, ImageTransform } from '../types.js'; @@ -28,6 +28,12 @@ type ImageConfig = Omit & { service: { entrypoint: string; config: T }; }; +type SrcSetValue = { + transform: ImageTransform; + descriptor?: string; + attributes?: Record; +}; + interface SharedServiceProps = Record> { /** * Return the URL to the endpoint or URL your images are generated from. @@ -38,6 +44,16 @@ interface SharedServiceProps = Record * */ getURL: (options: ImageTransform, imageConfig: ImageConfig) => string | Promise; + /** + * Generate additional `srcset` values for the image. + * + * While in most cases this is exclusively used for `srcset`, it can also be used in a more generic way to generate + * multiple variants of the same image. For instance, you can use this to generate multiple aspect ratios or multiple formats. + */ + getSrcSet?: ( + options: ImageTransform, + imageConfig: ImageConfig + ) => SrcSetValue[] | Promise; /** * Return any additional HTML attributes separate from `src` that your service requires to show the image properly. * @@ -174,6 +190,10 @@ export const baseService: Omit = { }); } + 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'; @@ -183,30 +203,15 @@ export const baseService: Omit = { // 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 = 'webp'; + options.format = DEFAULT_OUTPUT_FORMAT; } return options; }, getHTMLAttributes(options) { - let targetWidth = options.width; - let targetHeight = options.height; - if (isESMImportedImage(options.src)) { - const aspectRatio = options.src.width / options.src.height; - if (targetHeight && !targetWidth) { - // If we have a height but no width, use height to calculate the width - targetWidth = Math.round(targetHeight * aspectRatio); - } else if (targetWidth && !targetHeight) { - // If we have a width but no height, use width to calculate the height - targetHeight = Math.round(targetWidth / aspectRatio); - } else if (!targetWidth && !targetHeight) { - // If we have neither width or height, use the original image's dimensions - targetWidth = options.src.width; - targetHeight = options.src.height; - } - } - - const { src, width, height, format, quality, ...attributes } = options; + const { targetWidth, targetHeight } = getTargetDimensions(options); + const { src, width, height, format, quality, densities, widths, formats, ...attributes } = + options; return { ...attributes, @@ -216,6 +221,89 @@ export const baseService: Omit = { decoding: attributes.decoding ?? 'async', }; }, + getSrcSet(options) { + const srcSet: SrcSetValue[] = []; + const { targetWidth, targetHeight } = 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; + + // REFACTOR: Could we merge these two blocks? + if (densities) { + const densityValues = densities.map((density) => { + if (typeof density === 'number') { + return density; + } else { + return parseFloat(density); + } + }); + + 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); + }); + } 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); + } + + if (targetFormat !== options.format) { + srcSetValue.transform.format = targetFormat; + } + + srcSet.push(srcSetValue); + }); + } + + return srcSet; + }, getURL(options, imageConfig) { const searchParams = new URLSearchParams(); @@ -260,3 +348,28 @@ export const baseService: Omit = { return transform; }, }; + +function getTargetDimensions(options: ImageTransform) { + let targetWidth = options.width; + let targetHeight = options.height; + if (isESMImportedImage(options.src)) { + const aspectRatio = options.src.width / options.src.height; + if (targetHeight && !targetWidth) { + // If we have a height but no width, use height to calculate the width + targetWidth = Math.round(targetHeight * aspectRatio); + } else if (targetWidth && !targetHeight) { + // If we have a width but no height, use width to calculate the height + targetHeight = Math.round(targetWidth / aspectRatio); + } else if (!targetWidth && !targetHeight) { + // If we have neither width or height, use the original image's dimensions + targetWidth = options.src.width; + targetHeight = options.src.height; + } + } + + // TypeScript doesn't know this, but because of previous hooks we always know that targetWidth and targetHeight are defined + return { + targetWidth: targetWidth!, + targetHeight: targetHeight!, + }; +} diff --git a/packages/astro/src/assets/services/sharp.ts b/packages/astro/src/assets/services/sharp.ts index 0819ffcd13bf..21529913892c 100644 --- a/packages/astro/src/assets/services/sharp.ts +++ b/packages/astro/src/assets/services/sharp.ts @@ -33,6 +33,7 @@ const sharpService: LocalImageService = { getURL: baseService.getURL, parseURL: baseService.parseURL, getHTMLAttributes: baseService.getHTMLAttributes, + getSrcSet: baseService.getSrcSet, async transform(inputBuffer, transformOptions) { if (!sharp) sharp = await loadSharp(); @@ -49,9 +50,9 @@ const sharpService: LocalImageService = { // Never resize using both width and height at the same time, prioritizing width. if (transform.height && !transform.width) { - result.resize({ height: transform.height }); + result.resize({ height: Math.round(transform.height) }); } else if (transform.width) { - result.resize({ width: transform.width }); + result.resize({ width: Math.round(transform.width) }); } if (transform.format) { diff --git a/packages/astro/src/assets/services/squoosh.ts b/packages/astro/src/assets/services/squoosh.ts index 95c16b8d836b..5be5d40770c6 100644 --- a/packages/astro/src/assets/services/squoosh.ts +++ b/packages/astro/src/assets/services/squoosh.ts @@ -56,6 +56,7 @@ const service: LocalImageService = { getURL: baseService.getURL, parseURL: baseService.parseURL, getHTMLAttributes: baseService.getHTMLAttributes, + getSrcSet: baseService.getSrcSet, async transform(inputBuffer, transformOptions) { const transform: BaseServiceTransform = transformOptions as BaseServiceTransform; @@ -76,12 +77,12 @@ const service: LocalImageService = { if (transform.height && !transform.width) { operations.push({ type: 'resize', - height: transform.height, + height: Math.round(transform.height), }); } else if (transform.width) { operations.push({ type: 'resize', - width: transform.width, + width: Math.round(transform.width), }); } diff --git a/packages/astro/src/assets/types.ts b/packages/astro/src/assets/types.ts index 1712cda1cb85..43bfe0f53c5e 100644 --- a/packages/astro/src/assets/types.ts +++ b/packages/astro/src/assets/types.ts @@ -28,6 +28,12 @@ export interface ImageMetadata { orientation?: number; } +export interface SrcSetValue { + url: string; + descriptor?: string; + attributes?: Record; +} + /** * A yet to be resolved image transform. Used by `getImage` */ @@ -41,6 +47,8 @@ export type UnresolvedImageTransform = Omit & { export type ImageTransform = { src: ImageMetadata | string; width?: number | undefined; + widths?: number[] | undefined; + densities?: (number | `${number}x`)[] | undefined; height?: number | undefined; quality?: ImageQuality | undefined; format?: ImageOutputFormat | undefined; @@ -51,6 +59,10 @@ export interface GetImageResult { rawOptions: ImageTransform; options: ImageTransform; src: string; + srcSet: { + values: SrcSetValue[]; + attribute: string; + }; attributes: Record; } @@ -58,7 +70,7 @@ type ImageSharedProps = T & { /** * Width of the image, the value of this property will be used to assign the `width` property on the final `img` element. * - * For local images, this value will additionally be used to resize the image to the desired width, taking into account the original aspect ratio of the image. + * This value will additionally be used to resize the image to the desired width, taking into account the original aspect ratio of the image. * * **Example**: * ```astro @@ -85,7 +97,26 @@ type ImageSharedProps = T & { * ``` */ height?: number | `${number}`; -}; +} & ( + | { + /** + * A list of widths to generate images for. The value of this property will be used to assign the `srcset` property on the final `img` element. + * + * This attribute is incompatible with `densities`. + */ + widths?: number[]; + densities?: never; + } + | { + /** + * A list of pixel densities to generate images for. The value of this property will be used to assign the `srcset` property on the final `img` element. + * + * This attribute is incompatible with `widths`. + */ + densities?: (number | `${number}x`)[]; + widths?: never; + } + ); export type LocalImageProps = ImageSharedProps & { /** diff --git a/packages/astro/src/assets/utils/transformToPath.ts b/packages/astro/src/assets/utils/transformToPath.ts index d5535137be04..d9e11f3222fe 100644 --- a/packages/astro/src/assets/utils/transformToPath.ts +++ b/packages/astro/src/assets/utils/transformToPath.ts @@ -16,8 +16,8 @@ export function propsToFilename(transform: ImageTransform, hash: string) { } export function hashTransform(transform: ImageTransform, imageService: string) { - // take everything from transform except alt, which is not used in the hash - const { alt, ...rest } = transform; + // 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)); } diff --git a/packages/astro/src/assets/vite-plugin-assets.ts b/packages/astro/src/assets/vite-plugin-assets.ts index fd3ca2c32193..ea576fb1aa27 100644 --- a/packages/astro/src/assets/vite-plugin-assets.ts +++ b/packages/astro/src/assets/vite-plugin-assets.ts @@ -57,6 +57,7 @@ export default function assets({ export { getConfiguredImageService, isLocalService } from "astro/assets"; import { getImage as getImageInternal } from "astro/assets"; export { default as Image } from "astro/components/Image.astro"; + export { default as Picture } from "astro/components/Picture.astro"; export const imageConfig = ${JSON.stringify(settings.config.image)}; export const assetsDir = new URL(${JSON.stringify( diff --git a/packages/astro/src/core/errors/errors-data.ts b/packages/astro/src/core/errors/errors-data.ts index d805bb6ff7ce..05ccc1125924 100644 --- a/packages/astro/src/core/errors/errors-data.ts +++ b/packages/astro/src/core/errors/errors-data.ts @@ -621,6 +621,21 @@ export const ExpectedImageOptions = { `Expected getImage() parameter to be an object. Received \`${options}\`.`, } satisfies ErrorData; +/** + * @docs + * @see + * - [Images](https://docs.astro.build/en/guides/images/) + * @description + * Only one of `densities` or `widths` can be specified. Those attributes are used to construct a `srcset` attribute, which cannot have both `x` and `w` descriptors. + */ +export const IncompatibleDescriptorOptions = { + name: 'IncompatibleDescriptorOptions', + title: 'Cannot set both `densities` and `widths`', + message: + "Only one of `densities` or `widths` can be specified. In most cases, you'll probably want to use only `widths` if you require specific widths.", + hint: 'Those attributes are used to construct a `srcset` attribute, which cannot have both `x` and `w` descriptors.', +} satisfies ErrorData; + /** * @docs * @see diff --git a/packages/astro/test/core-image.test.js b/packages/astro/test/core-image.test.js index 653ad5dfdc49..93c4fd023268 100644 --- a/packages/astro/test/core-image.test.js +++ b/packages/astro/test/core-image.test.js @@ -2,6 +2,7 @@ import { expect } from 'chai'; import * as cheerio from 'cheerio'; import { basename } from 'node:path'; import { Writable } from 'node:stream'; +import parseSrcset from 'parse-srcset'; import { removeDir } from '../dist/core/fs/index.js'; import { Logger } from '../dist/core/logger/core.js'; import testAdapter from './test-adapter.js'; @@ -188,6 +189,36 @@ describe('astro:image', () => { expect(res.status).to.equal(200); expect(res.headers.get('content-type')).to.equal('image/avif'); }); + + it('has a working Picture component', async () => { + let res = await fixture.fetch('/picturecomponent'); + let html = await res.text(); + $ = cheerio.load(html); + + // Densities + let $img = $('#picture-density-2-format img'); + let $picture = $('#picture-density-2-format picture'); + let $source = $('#picture-density-2-format source'); + expect($img).to.have.a.lengthOf(1); + expect($picture).to.have.a.lengthOf(1); + expect($source).to.have.a.lengthOf(2); + + const srcset = parseSrcset($source.attr('srcset')); + expect(srcset.every((src) => src.url.startsWith('/_image'))).to.equal(true); + expect(srcset.map((src) => src.d)).to.deep.equal([undefined, 2]); + + // Widths + $img = $('#picture-widths img'); + $picture = $('#picture-widths picture'); + $source = $('#picture-widths source'); + expect($img).to.have.a.lengthOf(1); + expect($picture).to.have.a.lengthOf(1); + expect($source).to.have.a.lengthOf(1); + + 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]); + }); }); describe('vite-isms', () => { @@ -702,6 +733,26 @@ describe('astro:image', () => { expect(data).to.be.an.instanceOf(Buffer); }); + it('Picture component images are written', async () => { + const html = await fixture.readFile('/picturecomponent/index.html'); + const $ = cheerio.load(html); + let $img = $('img'); + let $source = $('source'); + + expect($img).to.have.a.lengthOf(1); + expect($source).to.have.a.lengthOf(2); + + const srcset = parseSrcset($source.attr('srcset')); + let hasExistingSrc = await Promise.all( + srcset.map(async (src) => { + const data = await fixture.readFile(src.url, null); + return data instanceof Buffer; + }) + ); + + expect(hasExistingSrc.every((src) => src === true)).to.deep.equal(true); + }); + it('markdown images are written', async () => { const html = await fixture.readFile('/post/index.html'); const $ = cheerio.load(html); diff --git a/packages/astro/test/fixtures/core-image-ssg/src/pages/picturecomponent.astro b/packages/astro/test/fixtures/core-image-ssg/src/pages/picturecomponent.astro new file mode 100644 index 000000000000..8515538d49f8 --- /dev/null +++ b/packages/astro/test/fixtures/core-image-ssg/src/pages/picturecomponent.astro @@ -0,0 +1,6 @@ +--- +import { Picture } from "astro:assets"; +import myImage from "../assets/penguin1.jpg"; +--- + + diff --git a/packages/astro/test/fixtures/core-image/src/pages/picturecomponent.astro b/packages/astro/test/fixtures/core-image/src/pages/picturecomponent.astro new file mode 100644 index 000000000000..a849964bfaa3 --- /dev/null +++ b/packages/astro/test/fixtures/core-image/src/pages/picturecomponent.astro @@ -0,0 +1,12 @@ +--- +import { Picture } from "astro:assets"; +import myImage from "../assets/penguin1.jpg"; +--- + +
+ +
+ +
+ +
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 4d662e17b2d0..a2ac5d3f1180 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -753,6 +753,9 @@ importers: node-mocks-http: specifier: ^1.13.0 version: 1.13.0 + parse-srcset: + specifier: ^1.0.2 + version: 1.0.2 rehype-autolink-headings: specifier: ^6.1.1 version: 6.1.1 @@ -14747,6 +14750,10 @@ packages: resolution: {integrity: sha512-kBeTUtcj+SkyfaW4+KBe0HtsloBJ/mKTPoxpVdA57GZiPerREsUWJOhVj9anXweFiJkm5y8FG1sxFZkZ0SN6wg==} dev: false + /parse-srcset@1.0.2: + resolution: {integrity: sha512-/2qh0lav6CmI15FzA3i/2Bzk2zCgQhGMkvhOhKNcBVQ1ldgpbfiNTVslmooUmWJcADi1f1kIeynbDRVzNlfR6Q==} + dev: true + /parse5-htmlparser2-tree-adapter@7.0.0: resolution: {integrity: sha512-B77tOZrqqfUfnVcOrUvfdLbz4pu4RopLD/4vmu3HUPswwTA8OH0EMW9BlWR2B0RCoiZRAHEUu7IxeP1Pd1UU+g==} dependencies: From a964a141728e3639e11e930628ceef8de3bb75ca Mon Sep 17 00:00:00 2001 From: Princesseuh Date: Wed, 11 Oct 2023 15:56:21 +0000 Subject: [PATCH 2/3] [ci] format --- packages/astro/components/Picture.astro | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/astro/components/Picture.astro b/packages/astro/components/Picture.astro index 5ea2516d675e..00907f9d39af 100644 --- a/packages/astro/components/Picture.astro +++ b/packages/astro/components/Picture.astro @@ -48,8 +48,10 @@ if (fallbackImage.srcSet.values.length > 0) { { Object.entries(optimizedImages).map(([_, image]) => ( 0 ? ' , ' + image.srcSet.attribute : ''}`} - type={"image/" + image.options.format} + srcset={`${image.src}${ + image.srcSet.values.length > 0 ? ' , ' + image.srcSet.attribute : '' + }`} + type={'image/' + image.options.format} /> )) } From f999365b8248b8b14f3743e68a42d450d06acff3 Mon Sep 17 00:00:00 2001 From: Bjorn Lu Date: Wed, 11 Oct 2023 23:58:08 +0800 Subject: [PATCH 3/3] Fix markdown charset as utf-8 by default (#8795) --- .changeset/olive-laws-work.md | 5 + .../astro/src/runtime/server/render/page.ts | 5 + packages/astro/test/astro-basic.test.js | 118 +++++++++++++----- .../fixtures/astro-basic/astro.config.mjs | 3 +- .../test/fixtures/astro-basic/package.json | 1 + .../src/pages/chinese-encoding-md.md | 3 + .../src/pages/chinese-encoding-mdx.mdx | 3 + packages/astro/test/units/render/head.test.js | 6 +- packages/astro/test/units/render/jsx.test.js | 6 +- pnpm-lock.yaml | 3 + 10 files changed, 115 insertions(+), 38 deletions(-) create mode 100644 .changeset/olive-laws-work.md create mode 100644 packages/astro/test/fixtures/astro-basic/src/pages/chinese-encoding-md.md create mode 100644 packages/astro/test/fixtures/astro-basic/src/pages/chinese-encoding-mdx.mdx diff --git a/.changeset/olive-laws-work.md b/.changeset/olive-laws-work.md new file mode 100644 index 000000000000..483c759033ee --- /dev/null +++ b/.changeset/olive-laws-work.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fix markdown page charset to be utf-8 by default (same as Astro 2) diff --git a/packages/astro/src/runtime/server/render/page.ts b/packages/astro/src/runtime/server/render/page.ts index 2f4e87f5fc73..8bc5366cfb34 100644 --- a/packages/astro/src/runtime/server/render/page.ts +++ b/packages/astro/src/runtime/server/render/page.ts @@ -63,6 +63,11 @@ export async function renderPage( body = encoder.encode(body); headers.set('Content-Length', body.byteLength.toString()); } + // TODO: Revisit if user should manually set charset by themselves in Astro 4 + // This code preserves the existing behaviour for markdown pages since Astro 2 + if (route?.component.endsWith('.md')) { + headers.set('Content-Type', 'text/html; charset=utf-8'); + } const response = new Response(body, { ...init, headers }); return response; } diff --git a/packages/astro/test/astro-basic.test.js b/packages/astro/test/astro-basic.test.js index 3a75c1acc968..37a5693cf315 100644 --- a/packages/astro/test/astro-basic.test.js +++ b/packages/astro/test/astro-basic.test.js @@ -3,23 +3,31 @@ import * as cheerio from 'cheerio'; import { loadFixture } from './test-utils.js'; describe('Astro basics', () => { + /** @type {import('./test-utils').Fixture} */ let fixture; - let previewServer; before(async () => { fixture = await loadFixture({ root: './fixtures/astro-basic/', }); - await fixture.build(); - previewServer = await fixture.preview(); - }); - - // important: close preview server (free up port and connection) - after(async () => { - await previewServer.stop(); }); describe('build', () => { + let previewServer; + + before(async () => { + fixture = await loadFixture({ + root: './fixtures/astro-basic/', + }); + await fixture.build(); + previewServer = await fixture.preview(); + }); + + // important: close preview server (free up port and connection) + after(async () => { + await previewServer.stop(); + }); + it('Can load page', async () => { const html = await fixture.readFile(`/index.html`); const $ = cheerio.load(html); @@ -108,39 +116,87 @@ describe('Astro basics', () => { const $ = cheerio.load(html); expect($('#rendered-order').text()).to.eq('Rendered order: A, B'); }); - }); - it('Supports void elements whose name is a string (#2062)', async () => { - const html = await fixture.readFile('/input/index.html'); - const $ = cheerio.load(html); + it('renders markdown in utf-8 by default', async () => { + const html = await fixture.readFile('/chinese-encoding-md/index.html'); + const $ = cheerio.load(html); + expect($('h1').text()).to.equal('我的第一篇博客文章'); + }); + + it('renders MDX in utf-8 by default', async () => { + const html = await fixture.readFile('/chinese-encoding-mdx/index.html'); + const $ = cheerio.load(html); + expect($('h1').text()).to.equal('我的第一篇博客文章'); + }); + + it('Supports void elements whose name is a string (#2062)', async () => { + const html = await fixture.readFile('/input/index.html'); + const $ = cheerio.load(html); + + // + expect($('body > :nth-child(1)').prop('outerHTML')).to.equal(''); - // - expect($('body > :nth-child(1)').prop('outerHTML')).to.equal(''); + // + expect($('body > :nth-child(2)').prop('outerHTML')).to.equal(''); - // - expect($('body > :nth-child(2)').prop('outerHTML')).to.equal(''); + // + expect($('body > :nth-child(3)').prop('outerHTML')).to.equal(''); - // - expect($('body > :nth-child(3)').prop('outerHTML')).to.equal(''); + // + expect($('body > :nth-child(4)').prop('outerHTML')).to.equal( + '' + ); - // - expect($('body > :nth-child(4)').prop('outerHTML')).to.equal( - '' - ); + // textarea + expect($('body > :nth-child(5)').prop('outerHTML')).to.equal(''); + }); + + describe('preview', () => { + it('returns 200 for valid URLs', async () => { + const result = await fixture.fetch('/'); + expect(result.status).to.equal(200); + }); - // textarea - expect($('body > :nth-child(5)').prop('outerHTML')).to.equal(''); + it('returns 404 for invalid URLs', async () => { + const result = await fixture.fetch('/bad-url'); + expect(result.status).to.equal(404); + }); + }); }); - describe('preview', () => { - it('returns 200 for valid URLs', async () => { - const result = await fixture.fetch('/'); - expect(result.status).to.equal(200); + describe('development', () => { + /** @type {import('./test-utils').DevServer} */ + let devServer; + + before(async () => { + devServer = await fixture.startDevServer(); + }); + after(async () => { + await devServer.stop(); + }); + + it('Renders markdown in utf-8 by default', async () => { + const res = await fixture.fetch('/chinese-encoding-md'); + expect(res.status).to.equal(200); + const html = await res.text(); + const $ = cheerio.load(html); + expect($('h1').text()).to.equal('我的第一篇博客文章'); + const isUtf8 = + res.headers.get('content-type').includes('charset=utf-8') || + html.includes(''); + expect(isUtf8).to.be.true; }); - it('returns 404 for invalid URLs', async () => { - const result = await fixture.fetch('/bad-url'); - expect(result.status).to.equal(404); + it('Renders MDX in utf-8 by default', async () => { + const res = await fixture.fetch('/chinese-encoding-mdx'); + expect(res.status).to.equal(200); + const html = await res.text(); + const $ = cheerio.load(html); + expect($('h1').text()).to.equal('我的第一篇博客文章'); + const isUtf8 = + res.headers.get('content-type').includes('charset=utf-8') || + html.includes(''); + expect(isUtf8).to.be.true; }); }); }); diff --git a/packages/astro/test/fixtures/astro-basic/astro.config.mjs b/packages/astro/test/fixtures/astro-basic/astro.config.mjs index 1b2eb163d604..00158161f35d 100644 --- a/packages/astro/test/fixtures/astro-basic/astro.config.mjs +++ b/packages/astro/test/fixtures/astro-basic/astro.config.mjs @@ -1,9 +1,10 @@ import { defineConfig } from 'astro/config'; import preact from '@astrojs/preact'; +import mdx from '@astrojs/mdx'; // https://astro.build/config export default defineConfig({ - integrations: [preact()], + integrations: [preact(), mdx()], // make sure CLI flags have precedence server: () => ({ port: 4321 }) }); diff --git a/packages/astro/test/fixtures/astro-basic/package.json b/packages/astro/test/fixtures/astro-basic/package.json index ee968d78cc81..c2e0be656281 100644 --- a/packages/astro/test/fixtures/astro-basic/package.json +++ b/packages/astro/test/fixtures/astro-basic/package.json @@ -3,6 +3,7 @@ "version": "0.0.0", "private": true, "dependencies": { + "@astrojs/mdx": "workspace:*", "@astrojs/preact": "workspace:*", "astro": "workspace:*", "preact": "^10.17.1" diff --git a/packages/astro/test/fixtures/astro-basic/src/pages/chinese-encoding-md.md b/packages/astro/test/fixtures/astro-basic/src/pages/chinese-encoding-md.md new file mode 100644 index 000000000000..572b3c370b15 --- /dev/null +++ b/packages/astro/test/fixtures/astro-basic/src/pages/chinese-encoding-md.md @@ -0,0 +1,3 @@ +# 我的第一篇博客文章 + +发表于：2022-07-01 diff --git a/packages/astro/test/fixtures/astro-basic/src/pages/chinese-encoding-mdx.mdx b/packages/astro/test/fixtures/astro-basic/src/pages/chinese-encoding-mdx.mdx new file mode 100644 index 000000000000..572b3c370b15 --- /dev/null +++ b/packages/astro/test/fixtures/astro-basic/src/pages/chinese-encoding-mdx.mdx @@ -0,0 +1,3 @@ +# 我的第一篇博客文章 + +发表于：2022-07-01 diff --git a/packages/astro/test/units/render/head.test.js b/packages/astro/test/units/render/head.test.js index d2580e30de5b..3bb0fc84f924 100644 --- a/packages/astro/test/units/render/head.test.js +++ b/packages/astro/test/units/render/head.test.js @@ -90,7 +90,7 @@ describe('core/render', () => { const PageModule = createAstroModule(Page); const ctx = await createRenderContext({ - route: { type: 'page', pathname: '/index' }, + route: { type: 'page', pathname: '/index', component: 'src/pages/index.astro' }, request: new Request('http://example.com/'), links: [{ name: 'link', props: { rel: 'stylesheet', href: '/main.css' }, children: '' }], mod: PageModule, @@ -171,7 +171,7 @@ describe('core/render', () => { const PageModule = createAstroModule(Page); const ctx = await createRenderContext({ - route: { type: 'page', pathname: '/index' }, + route: { type: 'page', pathname: '/index', component: 'src/pages/index.astro' }, request: new Request('http://example.com/'), links: [{ name: 'link', props: { rel: 'stylesheet', href: '/main.css' }, children: '' }], env, @@ -218,7 +218,7 @@ describe('core/render', () => { const PageModule = createAstroModule(Page); const ctx = await createRenderContext({ - route: { type: 'page', pathname: '/index' }, + route: { type: 'page', pathname: '/index', component: 'src/pages/index.astro' }, request: new Request('http://example.com/'), links: [{ name: 'link', props: { rel: 'stylesheet', href: '/main.css' }, children: '' }], env, diff --git a/packages/astro/test/units/render/jsx.test.js b/packages/astro/test/units/render/jsx.test.js index 1464b5b0ce5b..65437cfc854a 100644 --- a/packages/astro/test/units/render/jsx.test.js +++ b/packages/astro/test/units/render/jsx.test.js @@ -45,7 +45,7 @@ describe('core/render', () => { const mod = createAstroModule(Page); const ctx = await createRenderContext({ - route: { type: 'page', pathname: '/index' }, + route: { type: 'page', pathname: '/index', component: 'src/pages/index.mdx' }, request: new Request('http://example.com/'), env, mod, @@ -91,7 +91,7 @@ describe('core/render', () => { const mod = createAstroModule(Page); const ctx = await createRenderContext({ - route: { type: 'page', pathname: '/index' }, + route: { type: 'page', pathname: '/index', component: 'src/pages/index.mdx' }, request: new Request('http://example.com/'), env, mod, @@ -117,7 +117,7 @@ describe('core/render', () => { const mod = createAstroModule(Page); const ctx = await createRenderContext({ - route: { type: 'page', pathname: '/index' }, + route: { type: 'page', pathname: '/index', component: 'src/pages/index.mdx' }, request: new Request('http://example.com/'), env, mod, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index a2ac5d3f1180..cac4d1d07666 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1750,6 +1750,9 @@ importers: packages/astro/test/fixtures/astro-basic: dependencies: + '@astrojs/mdx': + specifier: workspace:* + version: link:../../../../integrations/mdx '@astrojs/preact': specifier: workspace:* version: link:../../../../integrations/preact