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.

Sign up for GitHub

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

Add limitInputPixels option for sharp image service #9546

Merged
merged 8 commits into from
Jan 3, 2024
Merged

Add limitInputPixels option for sharp image service #9546

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1430af1
Add limitInputPixels option for sharp image service
bluwy Dec 29, 2023
57b7bf5
Merge branch 'main' into sharp-limit-input-pixels
bluwy Dec 29, 2023
15c8980
Fix types
bluwy Dec 29, 2023
f381c90
Merge branch 'main' into sharp-limit-input-pixels
ematipico Jan 2, 2024
a335361
Merge branch 'main' into sharp-limit-input-pixels
bluwy Jan 3, 2024
5cc9007
Update docs
bluwy Jan 3, 2024
90fc981
Merge branch 'main' into sharp-limit-input-pixels
ematipico Jan 3, 2024
b316915
Merge branch 'main' into sharp-limit-input-pixels
ematipico Jan 3, 2024
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/poor-apes-cheat.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': minor
---

Adds `limitInputPixels` option for the Sharp image service to be passed on to Sharp.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Adds `limitInputPixels` option for the Sharp image service to be passed on to Sharp.
Adds an option for the Sharp image service to allow large images to be processed. Set `limitInputPixels: false` to bypass the default image size limit:
```
// astro.mjs.config
example showing this config
```

(I'm assuming this is a config-wide option set, and not e.g. per-image.)

Depending on the appropriate code sample here, the same will need to be added to the astro types file. Since sharp is the default image service, someone might not have this in their config already, but they'd need to add it in order to pass the option. So, I'm thinking somewhere here: https://docs.astro.build/en/reference/configuration-reference/#imageservice but it would have to account for the fact that someone is only adding this NOT to actually change the image service used, but rather to configure the default one. (Since, this option is JUST for Sharp, and not the other services)

Depending on whatever's decided here, for an actual docs PR, we can even get away with just linking to the config option from here: https://docs.astro.build/en/guides/images/#default-image-service and don't need to explain it. We really should have a link there anyway! 😅

Sorry this requires a bit of figuring out, but I'll make sure I get up early tomorrow and am happy to help with it!

Copy link
Member Author

@bluwy bluwy Jan 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for the review! Yeah I have a rough idea to improve this now.

Thanks for linking me to the docs. I actually would've documented something like:

import { defineConfig, sharpImageService } from 'astro/config'

export default defineConfig({
  image: {
    service: sharpImageService({ limitInputPixels: false })
  }
})

But I think it's easier to keep the object format for now, so it applies more generally to other kinds of image services.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've updated the changesets and jsdoc. Let me know what you think!

3 changes: 2 additions & 1 deletion packages/astro/config.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ type ViteUserConfig = import('vite').UserConfig;
type ViteUserConfigFn = import('vite').UserConfigFn;
type AstroUserConfig = import('./dist/@types/astro.js').AstroUserConfig;
type ImageServiceConfig = import('./dist/@types/astro.js').ImageServiceConfig;
type SharpImageServiceConfig = import('./dist/assets/services/sharp.js').SharpImageServiceConfig;

/**
* See the full Astro Configuration API Documentation
Expand All @@ -17,7 +18,7 @@ export function getViteConfig(config: ViteUserConfig): ViteUserConfigFn;
/**
* Return the configuration needed to use the Sharp-based image service
*/
export function sharpImageService(): ImageServiceConfig;
export function sharpImageService(config?: SharpImageServiceConfig): ImageServiceConfig;

/**
* Return the configuration needed to use the Squoosh-based image service
Expand Down
4 changes: 2 additions & 2 deletions packages/astro/config.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
export { defineConfig, getViteConfig } from './dist/config/index.js';

export function sharpImageService() {
export function sharpImageService(config = {}) {
return {
entrypoint: 'astro/assets/services/sharp',
config: {},
config,
};
}

Expand Down
17 changes: 14 additions & 3 deletions packages/astro/src/assets/services/sharp.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,13 @@ import {
type LocalImageService,
} from './service.js';

export interface SharpImageServiceConfig {
/**
* The `limitInputPixels` option passed to Sharp. See https://sharp.pixelplumbing.com/api-constructor for more information
*/
limitInputPixels?: number;
}

let sharp: typeof import('sharp');

const qualityTable: Record<ImageQualityPreset, number> = {
Expand All @@ -28,13 +35,13 @@ async function loadSharp() {
return sharpImport;
}

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

const transform: BaseServiceTransform = transformOptions as BaseServiceTransform;
Expand All @@ -43,7 +50,11 @@ const sharpService: LocalImageService = {
// TODO: Sharp has some support for SVGs, we could probably support this once Sharp is the default and only service.
if (transform.format === 'svg') return { data: inputBuffer, format: 'svg' };

let result = sharp(inputBuffer, { failOnError: false, pages: -1 });
const result = sharp(inputBuffer, {
failOnError: false,
pages: -1,
limitInputPixels: config.service.config.limitInputPixels,
Princesseuh marked this conversation as resolved.
Show resolved Hide resolved
});

// always call rotate to adjust for EXIF data orientation
result.rotate();
Expand Down