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 1 commit
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
Prev Previous commit
Next Next commit
adding type definitions for the integration's use of globalThis
  • Loading branch information
Tony Sullivan committed Jul 18, 2022
commit dcf56e43e49cacc77b0567bc7201c3bf7b19e6c5
6 changes: 5 additions & 1 deletion packages/integrations/image/src/endpoints/dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@ import { lookup } from 'mrmime';
import { loadImage } from '../utils.js';

export const get: APIRoute = async ({ request }) => {
const loader = (globalThis as any)['@astrojs/image'].ssrLoader;
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)


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

try {
const url = new URL(request.url);
Expand Down
18 changes: 11 additions & 7 deletions packages/integrations/image/src/get-image.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,29 +105,33 @@ export async function getImage(
loader: ImageService,
transform: GetImageTransform
): Promise<ImageAttributes> {
(globalThis as any)['@astrojs/image'].loader = loader;
globalThis.astroImage.loader = loader;

const resolved = await resolveTransform(transform);

const attributes = await loader.getImageAttributes(resolved);

const isDev = (globalThis as any)['@astrojs/image'].command === 'dev';
const isDev = globalThis.astroImage.command === 'dev';
const isLocalImage = !isRemoteImage(resolved.src);

const _loader = isDev && isLocalImage ? (globalThis as any)['@astrojs/image'].ssrLoader : loader;
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);

// cache all images rendered to HTML
if (globalThis && (globalThis as any)['@astrojs/image'].addStaticImage) {
(globalThis as any)['@astrojs/image'].addStaticImage(resolved);
if (globalThis && globalThis.astroImage.addStaticImage) {
globalThis.astroImage.addStaticImage(resolved);
}

const src =
globalThis && (globalThis as any)['@astrojs/image'].filenameFormat
? (globalThis as any)['@astrojs/image'].filenameFormat(resolved, searchParams)
globalThis && globalThis.astroImage.filenameFormat
? globalThis.astroImage.filenameFormat(resolved, searchParams)
: `${ROUTE_PATTERN}?${searchParams.toString()}`;

return {
Expand Down
15 changes: 10 additions & 5 deletions packages/integrations/image/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export * from './get-image.js';
export * from './get-picture.js';

const createIntegration = (options: IntegrationOptions = {}): AstroIntegration => {
(globalThis as any)['@astrojs/image'] = {
globalThis.astroImage = {
ssrLoader: sharp
};

Expand Down Expand Up @@ -48,7 +48,7 @@ const createIntegration = (options: IntegrationOptions = {}): AstroIntegration =
hooks: {
'astro:config:setup': ({ command, config, injectRoute, updateConfig }) => {
_config = config;
(globalThis as any)['@astrojs/image'].command = command;
globalThis.astroImage.command = command;

// Always treat `astro dev` as SSR mode, even without an adapter
const mode = command === 'dev' || config.adapter ? 'ssr' : 'ssg';
Expand All @@ -57,12 +57,12 @@ 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)['@astrojs/image'].addStaticImage = (transform: TransformOptions) => {
globalThis.astroImage.addStaticImage = (transform: TransformOptions) => {
staticImages.set(propsToFilename(transform), transform);
};

// TODO: Add support for custom, user-provided filename format functions
(globalThis as any)['@astrojs/image'].filenameFormat = (
globalThis.astroImage.filenameFormat = (
transform: TransformOptions,
searchParams: URLSearchParams
) => {
Expand All @@ -89,7 +89,12 @@ const createIntegration = (options: IntegrationOptions = {}): AstroIntegration =
},
'astro:build:done': async ({ dir }) => {
for await (const [filename, transform] of staticImages) {
const loader = (globalThis as any)['@astrojs/image'].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