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

Improving support for third-party hosted image services #3957

Merged
merged 6 commits into from
Jul 18, 2022
Merged
Show file tree
Hide file tree
Changes from 5 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
5 changes: 5 additions & 0 deletions .changeset/purple-vans-bake.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@astrojs/image': patch
---

Improves the `astro dev` experience when using a third-party hosted image service
2 changes: 2 additions & 0 deletions packages/integrations/image/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ The included `sharp` transformer supports resizing images and encoding them to d

The intergration can be configured to run with a different image service, either a hosted image service or a full image transformer that runs locally in your build or SSR deployment.

> During development, local images may not have been published yet and would not be available to hosted image services. Local images will always use the built-in `sharp` service when using `astro dev`.

There are currently no other configuration options for the `@astrojs/image` integration. Please [open an issue](https://github.com/withastro/astro/issues/new/choose) if you have a compelling use case to share.

<details>
Expand Down
4 changes: 2 additions & 2 deletions packages/integrations/image/src/endpoints/dev.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import type { APIRoute } from 'astro';
import { lookup } from 'mrmime';
// @ts-ignore
import loader from 'virtual:image-loader';
import { loadImage } from '../utils.js';

export const get: APIRoute = async ({ request }) => {
const loader = globalThis.astroImage.ssrLoader;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix pt. 1 - getImage() in dev will now only build a _image? src for local images. The endpoint can always use the ssrLoader global (currently defaults to the built-in sharp service)


try {
const url = new URL(request.url);
const transform = loader.parseTransform(url.searchParams);
Expand Down
27 changes: 18 additions & 9 deletions packages/integrations/image/src/get-image.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
OutputFormat,
TransformOptions,
} from './types.js';
import { parseAspectRatio } from './utils.js';
import { isRemoteImage, parseAspectRatio } from './utils.js';

export interface GetImageTransform extends Omit<TransformOptions, 'src'> {
src: string | ImageMetadata | Promise<{ default: ImageMetadata }>;
Expand Down Expand Up @@ -105,23 +105,32 @@ export async function getImage(
loader: ImageService,
transform: GetImageTransform
): Promise<ImageAttributes> {
(globalThis as any).loader = loader;
globalThis.astroImage.loader = loader;

const resolved = await resolveTransform(transform);

const attributes = await loader.getImageAttributes(resolved);

const isDev = globalThis.astroImage.command === 'dev';
const isLocalImage = !isRemoteImage(resolved.src);

const _loader = isDev && isLocalImage ? globalThis.astroImage.ssrLoader : loader;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix pt. 2 - getImage() will always use the ssrLoader for local images during development

This makes sure that we don't build a src to a third-party service like Cloudinary for local images that aren't published yet


if (!_loader) {
throw new Error('@astrojs/image: loader not found!');
}

// For SSR services, build URLs for the injected route
if (isSSRService(loader)) {
const { searchParams } = loader.serializeTransform(resolved);
if (isSSRService(_loader)) {
const { searchParams } = _loader.serializeTransform(resolved);

// cache all images rendered to HTML
if (globalThis && (globalThis as any).addStaticImage) {
(globalThis as any)?.addStaticImage(resolved);
if (globalThis?.astroImage) {
globalThis.astroImage.addStaticImage(resolved);
}

const src =
globalThis && (globalThis as any).filenameFormat
? (globalThis as any).filenameFormat(resolved, searchParams)
const src = globalThis?.astroImage
? globalThis.astroImage.filenameFormat(resolved, searchParams)
: `${ROUTE_PATTERN}?${searchParams.toString()}`;

return {
Expand Down
24 changes: 20 additions & 4 deletions packages/integrations/image/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import fs from 'fs/promises';
import path from 'path';
import { fileURLToPath } from 'url';
import { OUTPUT_DIR, PKG_NAME, ROUTE_PATTERN } from './constants.js';
import sharp from './loaders/sharp.js';
import { IntegrationOptions, TransformOptions } from './types.js';
import {
ensureDir,
Expand Down Expand Up @@ -51,15 +52,15 @@ const createIntegration = (options: IntegrationOptions = {}): AstroIntegration =

// Used to cache all images rendered to HTML
// Added to globalThis to share the same map in Node and Vite
(globalThis as any).addStaticImage = (transform: TransformOptions) => {
function addStaticImage(transform: TransformOptions) {
staticImages.set(propsToFilename(transform), transform);
};

// TODO: Add support for custom, user-provided filename format functions
(globalThis as any).filenameFormat = (
function filenameFormat(
transform: TransformOptions,
searchParams: URLSearchParams
) => {
) {
if (mode === 'ssg') {
return isRemoteImage(transform.src)
? path.join(OUTPUT_DIR, path.basename(propsToFilename(transform)))
Expand All @@ -73,6 +74,16 @@ const createIntegration = (options: IntegrationOptions = {}): AstroIntegration =
}
};

// Initialize the integration's globalThis namespace
// This is needed to share scope between Node and Vite
globalThis.astroImage = {
loader: undefined, // initialized in first getImage() call
ssrLoader: sharp,
command,
addStaticImage,
filenameFormat,
}

if (mode === 'ssr') {
injectRoute({
pattern: ROUTE_PATTERN,
Expand All @@ -83,7 +94,12 @@ const createIntegration = (options: IntegrationOptions = {}): AstroIntegration =
},
'astro:build:done': async ({ dir }) => {
for await (const [filename, transform] of staticImages) {
const loader = (globalThis as any).loader;
const loader = globalThis.astroImage.loader;

if (!loader || !('transform' in loader)) {
// this should never be hit, how was a staticImage added without an SSR service?
return;
}

let inputBuffer: Buffer | undefined = undefined;
let outputFile: string;
Expand Down
12 changes: 12 additions & 0 deletions packages/integrations/image/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,18 @@
export type { Image, Picture } from '../components/index.js';
export * from './index.js';

interface ImageIntegration {
loader?: ImageService;
ssrLoader: SSRImageService;
command: 'dev' | 'build';
addStaticImage: (transform: TransformOptions) => void;
filenameFormat: (transform: TransformOptions, searchParams: URLSearchParams) => string;
}

declare global {
var astroImage: ImageIntegration;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Making typescript happy with a globalThis definition 🎉

}

export type InputFormat =
| 'heic'
| 'heif'
Expand Down