Skip to content

Commit

Permalink
Misc improvements for astro:assets (#7759)
Browse files Browse the repository at this point in the history
  • Loading branch information
Princesseuh committed Jul 21, 2023
1 parent 16a4152 commit 1792737
Show file tree
Hide file tree
Showing 14 changed files with 50 additions and 100 deletions.
5 changes: 5 additions & 0 deletions .changeset/hot-buckets-tie.md
@@ -0,0 +1,5 @@
---
'astro': patch
---

Fix SharedImageService's types not properly reflecting that image services hooks can be async
2 changes: 1 addition & 1 deletion packages/astro/src/assets/image-endpoint.ts
Expand Up @@ -22,7 +22,7 @@ async function loadRemoteImage(src: URL) {
}

/**
* Endpoint used in SSR to serve optimized images
* Endpoint used in dev and SSR to serve optimized images by the base image services
*/
export const get: APIRoute = async ({ request }) => {
try {
Expand Down
4 changes: 2 additions & 2 deletions packages/astro/src/assets/internal.ts
Expand Up @@ -38,10 +38,10 @@ export async function getImage(

const service = await getConfiguredImageService();
const validatedOptions = service.validateOptions
? service.validateOptions(options, serviceConfig)
? await service.validateOptions(options, serviceConfig)
: options;

let imageURL = service.getURL(validatedOptions, serviceConfig);
let imageURL = await service.getURL(validatedOptions, serviceConfig);

// 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) {
Expand Down
16 changes: 11 additions & 5 deletions packages/astro/src/assets/services/service.ts
Expand Up @@ -32,7 +32,7 @@ interface SharedServiceProps {
* For external services, this should point to the URL your images are coming from, for instance, `/_vercel/image`
*
*/
getURL: (options: ImageTransform, serviceConfig: Record<string, any>) => string;
getURL: (options: ImageTransform, serviceConfig: Record<string, any>) => string | Promise<string>;
/**
* Return any additional HTML attributes separate from `src` that your service requires to show the image properly.
*
Expand All @@ -42,7 +42,7 @@ interface SharedServiceProps {
getHTMLAttributes?: (
options: ImageTransform,
serviceConfig: Record<string, any>
) => Record<string, any>;
) => Record<string, any> | Promise<Record<string, any>>;
/**
* Validate and return the options passed by the user.
*
Expand All @@ -51,7 +51,10 @@ interface SharedServiceProps {
*
* 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, serviceConfig: Record<string, any>) => ImageTransform;
validateOptions?: (
options: ImageTransform,
serviceConfig: Record<string, any>
) => ImageTransform | Promise<ImageTransform>;
}

export type ExternalImageService = SharedServiceProps;
Expand All @@ -63,11 +66,14 @@ export type LocalImageTransform = {

export interface LocalImageService extends SharedServiceProps {
/**
* Parse the requested parameters passed in the URL from `getURL` back into an object to be used later by `transform`
* Parse the requested parameters passed in the URL from `getURL` back into an object to be used later by `transform`.
*
* In most cases, this will get query parameters using, for example, `params.get('width')` and return those.
*/
parseURL: (url: URL, serviceConfig: Record<string, any>) => LocalImageTransform | undefined;
parseURL: (
url: URL,
serviceConfig: Record<string, any>
) => LocalImageTransform | undefined | Promise<LocalImageTransform> | Promise<undefined>;
/**
* Performs the image transformations on the input image and returns both the binary data and
* final image format of the optimized image.
Expand Down
3 changes: 3 additions & 0 deletions packages/astro/src/assets/utils/index.ts
@@ -1 +1,4 @@
export { emitESMImage } from './emitAsset.js';
export { imageMetadata } from './metadata.js';
export { getOrigQueryParams } from './queryParams.js';
export { hashTransform, propsToFilename } from './transformToPath.js';
70 changes: 0 additions & 70 deletions packages/astro/src/assets/vite-plugin-assets.ts
@@ -1,8 +1,5 @@
import { bold } from 'kleur/colors';
import MagicString from 'magic-string';
import mime from 'mime/lite.js';
import fs from 'node:fs/promises';
import { Readable } from 'node:stream';
import { fileURLToPath } from 'node:url';
import type * as vite from 'vite';
import { normalizePath } from 'vite';
Expand All @@ -16,10 +13,7 @@ import {
} from '../core/path.js';
import { VIRTUAL_MODULE_ID, VIRTUAL_SERVICE_ID } from './consts.js';
import { isESMImportedImage } from './internal.js';
import { isLocalService } from './services/service.js';
import { emitESMImage } from './utils/emitAsset.js';
import { imageMetadata } from './utils/metadata.js';
import { getOrigQueryParams } from './utils/queryParams.js';
import { hashTransform, propsToFilename } from './utils/transformToPath.js';

const resolvedVirtualModuleId = '\0' + VIRTUAL_MODULE_ID;
Expand Down Expand Up @@ -96,70 +90,6 @@ export default function assets({
`;
}
},
// Handle serving images during development
configureServer(server) {
server.middlewares.use(async (req, res, next) => {
if (req.url?.startsWith('/_image')) {
// If the currently configured service isn't a local service, we don't need to do anything here.
// TODO: Support setting a specific service through a prop on Image / a parameter in getImage
if (!isLocalService(globalThis.astroAsset.imageService)) {
return next();
}

const url = new URL(req.url, 'file:');
if (!url.searchParams.has('href')) {
return next();
}

const filePath = url.searchParams.get('href')?.slice('/@fs'.length);
const filePathURL = new URL('.' + filePath, 'file:');
const file = await fs.readFile(filePathURL);

// Get the file's metadata from the URL
let meta = getOrigQueryParams(filePathURL.searchParams);

// If we don't have them (ex: the image came from Markdown, let's calculate them again)
if (!meta) {
meta = await imageMetadata(filePathURL, file);

if (!meta) {
return next();
}
}

const transform = await globalThis.astroAsset.imageService.parseURL(
url,
settings.config.image.service.config
);

if (transform === undefined) {
error(logging, 'image', `Failed to parse transform for ${url}`);
}

// if no transforms were added, the original file will be returned as-is
let data = file;
let format: string = meta.format;

if (transform) {
const result = await globalThis.astroAsset.imageService.transform(
file,
transform,
settings.config.image.service.config
);
data = result.data;
format = result.format;
}

res.setHeader('Content-Type', mime.getType(format) ?? `image/${format}`);
res.setHeader('Cache-Control', 'max-age=360000');

const stream = Readable.from(data);
return stream.pipe(res);
}

return next();
});
},
buildStart() {
if (mode != 'build') {
return;
Expand Down
4 changes: 3 additions & 1 deletion packages/astro/src/cli/load-settings.ts
Expand Up @@ -26,9 +26,11 @@ export async function loadSettings({ cmd, flags, logging }: LoadSettingsOptions)
await handleConfigError(e, { cmd, cwd: root, flags, logging });
return {} as any;
});

const mode = cmd === 'build' ? 'build' : 'dev';
if (!initialAstroConfig) return;
telemetry.record(event.eventCliSession(cmd, initialUserConfig, flags));
return createSettings(initialAstroConfig, root);
return createSettings(initialAstroConfig, mode, root);
}

export async function handleConfigError(
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/config/index.ts
Expand Up @@ -35,7 +35,7 @@ export function getViteConfig(inlineConfig: UserConfig) {
level: 'info',
};
const { astroConfig: config } = await openConfig({ cmd });
const settings = createSettings(config, inlineConfig.root);
const settings = createSettings(config, cmd, inlineConfig.root);
await runHookConfigSetup({ settings, command: cmd, logging });
const viteConfig = await createVite(
{
Expand Down
14 changes: 9 additions & 5 deletions packages/astro/src/core/config/settings.ts
Expand Up @@ -14,7 +14,7 @@ import { createDefaultDevConfig } from './config.js';
import { AstroTimer } from './timer.js';
import { loadTSConfig } from './tsconfig.js';

export function createBaseSettings(config: AstroConfig): AstroSettings {
export function createBaseSettings(config: AstroConfig, mode: 'build' | 'dev'): AstroSettings {
const { contentDir } = getContentPaths(config);
return {
config,
Expand All @@ -23,7 +23,7 @@ export function createBaseSettings(config: AstroConfig): AstroSettings {

adapter: undefined,
injectedRoutes:
config.experimental.assets && isServerLikeOutput(config)
config.experimental.assets && (isServerLikeOutput(config) || mode === 'dev')
? [{ pattern: '/_image', entryPoint: 'astro/assets/image-endpoint', prerender: false }]
: [],
pageExtensions: ['.astro', '.html', ...SUPPORTED_MARKDOWN_FILE_EXTENSIONS],
Expand Down Expand Up @@ -108,9 +108,13 @@ export function createBaseSettings(config: AstroConfig): AstroSettings {
};
}

export function createSettings(config: AstroConfig, cwd?: string): AstroSettings {
export function createSettings(
config: AstroConfig,
mode: 'build' | 'dev',
cwd?: string
): AstroSettings {
const tsconfig = loadTSConfig(cwd);
const settings = createBaseSettings(config);
const settings = createBaseSettings(config, mode);

const watchFiles = tsconfig?.exists ? [tsconfig.path, ...tsconfig.extendedPaths] : [];

Expand All @@ -132,5 +136,5 @@ export async function createDefaultDevSettings(
root = fileURLToPath(root);
}
const config = await createDefaultDevConfig(userConfig, root);
return createBaseSettings(config);
return createBaseSettings(config, 'dev');
}
2 changes: 1 addition & 1 deletion packages/astro/src/core/dev/restart.ts
Expand Up @@ -92,7 +92,7 @@ export async function restartContainer({
});
info(logging, 'astro', logMsg + '\n');
let astroConfig = newConfig.astroConfig;
const settings = createSettings(astroConfig, resolvedRoot);
const settings = createSettings(astroConfig, 'dev', resolvedRoot);
await close();
return {
container: await createRestartedContainer(container, settings, needsStart),
Expand Down
14 changes: 7 additions & 7 deletions packages/astro/test/test-utils.js
Expand Up @@ -138,8 +138,8 @@ export async function loadFixture(inlineConfig) {
* the `AstroSettings`. This function helps to create a fresh settings object that is used by the
* command functions below to prevent tests from polluting each other.
*/
const getSettings = async () => {
let settings = createSettings(config, fileURLToPath(cwd));
const getSettings = async (mode) => {
let settings = createSettings(config, mode, fileURLToPath(cwd));
if (config.integrations.find((integration) => integration.name === '@astrojs/mdx')) {
// Enable default JSX integration. It needs to come first, so unshift rather than push!
const { default: jsxRenderer } = await import('astro/jsx/renderer.js');
Expand Down Expand Up @@ -179,15 +179,15 @@ export async function loadFixture(inlineConfig) {
return {
build: async (opts = {}) => {
process.env.NODE_ENV = 'production';
return build(await getSettings(), { logging, ...opts });
return build(await getSettings('build'), { logging, ...opts });
},
sync: async (opts) => sync(await getSettings(), { logging, fs, ...opts }),
sync: async (opts) => sync(await getSettings('build'), { logging, fs, ...opts }),
check: async (opts) => {
return await check(await getSettings(), { logging, ...opts });
return await check(await getSettings('build'), { logging, ...opts });
},
startDevServer: async (opts = {}) => {
process.env.NODE_ENV = 'development';
devServer = await dev(await getSettings(), { logging, ...opts });
devServer = await dev(await getSettings('dev'), { logging, ...opts });
config.server.host = parseAddressToHost(devServer.address.address); // update host
config.server.port = devServer.address.port; // update port
return devServer;
Expand All @@ -209,7 +209,7 @@ export async function loadFixture(inlineConfig) {
},
preview: async (opts = {}) => {
process.env.NODE_ENV = 'production';
const previewServer = await preview(await getSettings(), { logging, ...opts });
const previewServer = await preview(await getSettings('build'), { logging, ...opts });
config.server.host = parseAddressToHost(previewServer.host); // update host
config.server.port = previewServer.port; // update port
return previewServer;
Expand Down
4 changes: 2 additions & 2 deletions packages/astro/test/units/config/format.test.js
@@ -1,7 +1,7 @@
import { expect } from 'chai';

import { createSettings, openConfig } from '../../../dist/core/config/index.js';
import { runInContainer } from '../../../dist/core/dev/index.js';
import { openConfig, createSettings } from '../../../dist/core/config/index.js';
import { createFs, defaultLogging } from '../test-utils.js';

const root = new URL('../../fixtures/tailwindcss-ts/', import.meta.url);
Expand All @@ -27,7 +27,7 @@ describe('Astro config formats', () => {
logging: defaultLogging,
fsMod: fs,
});
const settings = createSettings(astroConfig);
const settings = createSettings(astroConfig, 'dev');

await runInContainer({ fs, root, settings }, () => {
expect(true).to.equal(
Expand Down
Expand Up @@ -10,7 +10,7 @@ const logging = defaultLogging;

async function sync({ fs, config = {} }) {
const astroConfig = await validateConfig(config, fileURLToPath(root), 'prod');
const settings = createSettings(astroConfig, fileURLToPath(root));
const settings = createSettings(astroConfig, 'build', fileURLToPath(root));

return _sync(settings, { logging, fs });
}
Expand Down
8 changes: 4 additions & 4 deletions packages/astro/test/units/dev/restart.test.js
Expand Up @@ -2,6 +2,7 @@ import { expect } from 'chai';
import * as cheerio from 'cheerio';
import { fileURLToPath } from 'node:url';

import { createSettings, openConfig } from '../../../dist/core/config/index.js';
import {
createContainerWithAutomaticRestart,
isStarted,
Expand All @@ -13,7 +14,6 @@ import {
defaultLogging,
triggerFSEvent,
} from '../test-utils.js';
import { createSettings, openConfig } from '../../../dist/core/config/index.js';

const root = new URL('../../fixtures/alias/', import.meta.url);

Expand Down Expand Up @@ -133,7 +133,7 @@ describe('dev container restarts', () => {
cmd: 'dev',
logging: defaultLogging,
});
const settings = createSettings(astroConfig);
const settings = createSettings(astroConfig, 'dev');

let restart = await createContainerWithAutomaticRestart({
params: { fs, root, settings },
Expand Down Expand Up @@ -167,7 +167,7 @@ describe('dev container restarts', () => {
cmd: 'dev',
logging: defaultLogging,
});
const settings = createSettings(astroConfig, fileURLToPath(root));
const settings = createSettings(astroConfig, 'dev', fileURLToPath(root));

let restart = await createContainerWithAutomaticRestart({
params: { fs, root, settings },
Expand Down Expand Up @@ -199,7 +199,7 @@ describe('dev container restarts', () => {
cmd: 'dev',
logging: defaultLogging,
});
const settings = createSettings(astroConfig, fileURLToPath(root));
const settings = createSettings(astroConfig, 'dev', fileURLToPath(root));

let restart = await createContainerWithAutomaticRestart({
params: { fs, root, settings },
Expand Down

0 comments on commit 1792737

Please sign in to comment.