Skip to content

Commit

Permalink
Merge branch 'main' into fix-cc-err-empty-md
Browse files Browse the repository at this point in the history
  • Loading branch information
natemoo-re authored Mar 17, 2023
2 parents 3d925de + 66858f1 commit c86e4af
Show file tree
Hide file tree
Showing 11 changed files with 106 additions and 51 deletions.
6 changes: 6 additions & 0 deletions .changeset/tidy-dryers-add.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'astro': patch
'@astrojs/markdown-remark': patch
---

Add a `validateOptions` hook to the Image Service API in order to set default options and validate the passed options
5 changes: 5 additions & 0 deletions .changeset/warm-bananas-provide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Fix overflow title in error message
13 changes: 9 additions & 4 deletions packages/astro/src/assets/internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export async function getConfiguredImageService(): Promise<ImageService> {
}

interface GetImageResult {
rawOptions: ImageTransform;
options: ImageTransform;
src: string;
attributes: Record<string, any>;
Expand All @@ -50,17 +51,21 @@ interface GetImageResult {
*/
export async function getImage(options: ImageTransform): Promise<GetImageResult> {
const service = await getConfiguredImageService();
let imageURL = service.getURL(options);
const validatedOptions = service.validateOptions ? service.validateOptions(options) : options;

let imageURL = service.getURL(validatedOptions);

// 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) {
imageURL = globalThis.astroAsset.addStaticImage(options);
imageURL = globalThis.astroAsset.addStaticImage(validatedOptions);
}

return {
options,
rawOptions: options,
options: validatedOptions,
src: imageURL,
attributes: service.getHTMLAttributes !== undefined ? service.getHTMLAttributes(options) : {},
attributes:
service.getHTMLAttributes !== undefined ? service.getHTMLAttributes(validatedOptions) : {},
};
}

Expand Down
82 changes: 51 additions & 31 deletions packages/astro/src/assets/services/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,15 @@ interface SharedServiceProps {
* In most cases, you'll want to return directly what your user supplied you, minus the attributes that were used to generate the image.
*/
getHTMLAttributes?: (options: ImageTransform) => Record<string, any>;
/**
* Validate and return the options passed by the user.
*
* This method is useful to present errors to users who have entered invalid options.
* For instance, if they are missing a required property or have entered an invalid image format.
*
* This method should returns options, and can be used to set defaults (ex: a default output format to be used if the user didn't specify one.)
*/
validateOptions?: (options: ImageTransform) => ImageTransform;
}

export type ExternalImageService = SharedServiceProps;
Expand Down Expand Up @@ -69,7 +78,7 @@ export type BaseServiceTransform = {
src: string;
width?: number;
height?: number;
format?: string | null;
format: string;
quality?: string | null;
};

Expand All @@ -94,6 +103,45 @@ export type BaseServiceTransform = {
*
*/
export const baseService: Omit<LocalImageService, 'transform'> = {
validateOptions(options) {
if (!isESMImportedImage(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_INPUT_FORMATS.includes(options.src.format as any)) {
throw new AstroError({
...AstroErrorData.UnsupportedImageFormat,
message: AstroErrorData.UnsupportedImageFormat.message(
options.src.format,
options.src.src,
VALID_INPUT_FORMATS
),
});
}
}

// 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';
}

return options;
},
getHTMLAttributes(options) {
let targetWidth = options.width;
let targetHeight = options.height;
Expand Down Expand Up @@ -123,39 +171,11 @@ export const baseService: Omit<LocalImageService, 'transform'> = {
};
},
getURL(options: ImageTransform) {
// Both our currently available local services don't handle remote images, so we return the path as is.
if (!isESMImportedImage(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),
});
}

// Both our currently available local services don't handle remote images, so we return the path as is.
return options.src;
}

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

const searchParams = new URLSearchParams();
searchParams.append('href', options.src.src);

Expand All @@ -177,7 +197,7 @@ export const baseService: Omit<LocalImageService, 'transform'> = {
src: params.get('href')!,
width: params.has('w') ? parseInt(params.get('w')!) : undefined,
height: params.has('h') ? parseInt(params.get('h')!) : undefined,
format: params.get('f') as OutputFormat | null,
format: params.get('f') as OutputFormat,
quality: params.get('q'),
};

Expand Down
9 changes: 2 additions & 7 deletions packages/astro/src/assets/services/sharp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,14 @@ async function loadSharp() {
}

const sharpService: LocalImageService = {
validateOptions: baseService.validateOptions,
getURL: baseService.getURL,
parseURL: baseService.parseURL,
getHTMLAttributes: baseService.getHTMLAttributes,
async transform(inputBuffer, transformOptions) {
if (!sharp) sharp = await loadSharp();

const transform: BaseServiceTransform = transformOptions;

// 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 (!transform.format) {
transform.format = 'webp';
}
const transform: BaseServiceTransform = transformOptions as BaseServiceTransform;

let result = sharp(inputBuffer, { failOnError: false, pages: -1 });

Expand Down
6 changes: 2 additions & 4 deletions packages/astro/src/assets/services/squoosh.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,14 @@ const qualityTable: Record<Exclude<OutputFormat, 'png'>, Record<ImageQualityPres
};

const service: LocalImageService = {
validateOptions: baseService.validateOptions,
getURL: baseService.getURL,
parseURL: baseService.parseURL,
getHTMLAttributes: baseService.getHTMLAttributes,
async transform(inputBuffer, transformOptions) {
const transform: BaseServiceTransform = transformOptions as BaseServiceTransform;

let format = transform.format;
if (!format) {
format = 'webp';
}
let format = transform.format!;

const operations: Operation[] = [];

Expand Down
2 changes: 2 additions & 0 deletions packages/astro/src/core/errors/overlay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -412,13 +412,15 @@ const style = /* css */ `
padding: 24px;
display: flex;
justify-content: space-between;
gap: 1rem;
}
#code h2 {
font-family: var(--font-monospace);
color: var(--title-text);
font-size: 18px;
margin: 0;
overflow-wrap: anywhere;
}
#code .link {
Expand Down
16 changes: 13 additions & 3 deletions packages/astro/src/vite-plugin-markdown/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import type { Plugin } from 'vite';
import { normalizePath } from 'vite';
import type { AstroSettings } from '../@types/astro';
import { imageMetadata } from '../assets/index.js';
import type { ImageService } from '../assets/services/service';
import imageSize from '../assets/vendor/image-size/index.js';
import { AstroError, AstroErrorData, MarkdownError } from '../core/errors/index.js';
import type { LogOptions } from '../core/logger/core.js';
Expand Down Expand Up @@ -62,6 +63,8 @@ const astroJsxRuntimeModulePath = normalizePath(
export default function markdown({ settings, logging }: AstroPluginOptions): Plugin {
const markdownAssetMap = new Map<string, string>();

let imageService: ImageService | undefined = undefined;

async function resolveImage(this: PluginContext, fileId: string, path: string) {
const resolved = await this.resolve(path, fileId);
if (!resolved) return path;
Expand Down Expand Up @@ -93,7 +96,6 @@ export default function markdown({ settings, logging }: AstroPluginOptions): Plu
const rawFile = await fs.promises.readFile(fileId, 'utf-8');
const raw = safeMatter(rawFile, id);

let imageService = undefined;
if (settings.config.experimental.assets) {
imageService = (await import(settings.config.image.service)).default;
}
Expand Down Expand Up @@ -221,10 +223,18 @@ export default function markdown({ settings, logging }: AstroPluginOptions): Plu
}
const fileName = this.getFileName(hash);
image.src = npath.join(settings.config.base, fileName);
const optimized = globalThis.astroAsset.addStaticImage!({ src: image });

// TODO: This part recreates code we already have for content collection and normal ESM imports.
// It might be possible to refactor so it also uses `emitESMImage`? - erika, 2023-03-15
const options = { src: image };
const validatedOptions = imageService?.validateOptions
? imageService.validateOptions(options)
: options;

const optimized = globalThis.astroAsset.addStaticImage!(validatedOptions);
optimizedPaths.set(hash, optimized);
}
output.code = output.code.replace(/ASTRO_ASSET_MD_([0-9a-z]{8})/, (_str, hash) => {
output.code = output.code.replaceAll(/ASTRO_ASSET_MD_([0-9a-z]{8})/gm, (_str, hash) => {
const optimizedName = optimizedPaths.get(hash);
return optimizedName || this.getFileName(hash);
});
Expand Down
7 changes: 7 additions & 0 deletions packages/astro/test/core-image.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,13 @@ describe('astro:image', () => {
expect(data).to.be.an.instanceOf(Buffer);
});

it('writes out images to dist folder with proper extension if no format was passed', async () => {
const html = await fixture.readFile('/index.html');
const $ = cheerio.load(html);
const src = $('#local img').attr('src');
expect(src.endsWith('.webp')).to.be.true;
});

it('getImage() usage also written', async () => {
const html = await fixture.readFile('/get-image/index.html');
const $ = cheerio.load(html);
Expand Down
3 changes: 3 additions & 0 deletions packages/astro/test/fixtures/core-image/service.mjs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import squoosh from 'astro/assets/services/squoosh';

const service = {
validateOptions(options) {
return squoosh.validateOptions(options);
},
getURL(options) {
return squoosh.getURL(options);
},
Expand Down
8 changes: 6 additions & 2 deletions packages/markdown/remark/src/rehype-images.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,15 @@ export function rehypeImages(imageService: any, assetsDir: URL | undefined, getI
alt: node.properties.alt,
};

const imageURL = imageService.getURL(options);
const validatedOptions = imageService.validateOptions
? imageService.validateOptions(options)
: options;

const imageURL = imageService.getURL(validatedOptions);
node.properties = Object.assign(node.properties, {
src: imageURL,
...(imageService.getHTMLAttributes !== undefined
? imageService.getHTMLAttributes(options)
? imageService.getHTMLAttributes(validatedOptions)
: {}),
});
}
Expand Down

0 comments on commit c86e4af

Please sign in to comment.