feat(gatsby-plugin-sharp): create image sizes helper #27554

gillkyle · 2020-10-19T15:21:45Z

Description

TL:DR - utility that takes a bunch of info about an image and turns it into an array of numbers representing sizes we want to transform the image to

To decouple some of the behavior in gatsby-plugin-sharp and make the image resizing easier to test and tweak, this creates a utility that accepts a variety of arguments about an image, specifically:

file: some info like the path the image is stored at
imgDimensions: specific width/height of the file on disk that represents the maximum sizes that images can be transformed to
width/height/maxWidth/maxHeight: the desired dimensions of the image being transformed
layout: can be one of three types (fixed | fluid | constrained) and helps decide which branch of logic to follow to create sizes to transform to.
outputPixelDensities: array of desired multipliers to size up the image with for display on higher pixel density screens (should the defaults here be [1, 2, 3]?)
srcSetBreakpoints: array of pixel widths to additionally include in the output of sizes

I wrote up a variety of test cases to start trying to cover possibilities of how this could be used. I'm sure I've missed plenty however.

Related Issues

Related to work on this branch: https://github.com/gatsbyjs/gatsby/tree/feat/image-resolvers (hence the PR to merge into it haha)

packages/gatsby-plugin-sharp/src/utils.js

ascorbic · 2020-10-19T15:34:06Z

packages/gatsby-plugin-sharp/src/utils.js

+    if (!sizes.includes(maxWidth)) {
+      sizes.push(maxWidth)
+    }
+    sizes = sizes.sort((a, b) => a - b)


This is the default comparator, so you can just do sizes.sort()

So turns out this has this fun behavior of sorting by character which is almost right 😅 :

[0, 10, 190, 120, 500, 5, 1000000].sort() > [0, 10, 1000000, 120, 190, 5, 500]

vs.

[0, 10, 190, 120, 500, 5, 1000000].sort((a, b) => a - b) > [0, 5, 10, 120, 190, 500, 1000000]

ascorbic · 2020-10-19T15:38:37Z

packages/gatsby-plugin-sharp/src/__tests__/utils.js

+    expect(sizes).toEqual(expect.not.arrayContaining([1250, 1500]))
+  })
+
+  it(`should also include sizes from srcSetBreakpoints when outputPixelDensities are also passed in`, () => {


I think if we're passing in srcsetBreakpoints we should only use those

(Related to #27554 (comment))

Updated this, but a second set of eyes on the logic would be much appreciated 😄

ascorbic · 2020-10-19T15:40:09Z

packages/gatsby-plugin-sharp/src/utils.js

-      sizes = sizes.sort()
+    sizes = densities.map(density => density * fixedValue)
+    // add breakpoint set to sizes if they are specified
+    if (srcSetBreakpoints) {


srcSetBreakpoints should override everything else

Okay, in that case, do you think we should warn that any provided outputPixelDensities would have no effect if both are passed in? srcSetBreakpoints and outputPixelDensities approach the same kind of thing (custom specified sizes) from different angles. It seems a little confusing how they'd work together in that case.

The current resizing function adds the breakpoints to the array, but I can make it use them exclusively.

Yeah, I think it should be one or the other. A warning is a good idea. What do you reckon, @laurieontech ?

Ya, people might miss it, but always good to let them know when their code is ignored

Okay, I'll add a warning then, thanks! My suspicion is most people won't bother to use either one 😅 but if they need to, it should be clear.

Added a warning in a new commit, and updated the logic so only srcSetBreakpoints will be used when they are passed in.

wardpeet

I would like to drop 3x density by default as it creates large images without much benefits for the human eye, especially if they are used as thumbnails. I've linked a blog post from twitter below.

I like this cleanup! One thing I don't like is that we're not using it. Which means we're not really certain that this works as expected.

wardpeet · 2020-10-20T09:20:06Z

packages/gatsby-plugin-sharp/src/utils.js

+    Boolean(value)
+  )
+  if (ignoredParams.length) {
+    console.warn(


Please use the reporter

Suggested change

console.warn(

console.warn(

wardpeet · 2020-10-20T09:26:51Z

packages/gatsby-plugin-sharp/src/utils.js

+  return
+}
+
+const DEFAULT_PIXEL_DENSITIES = [0.25, 0.5, 1, 2, 3]


We should not enable density 3 by default, for constrained images it hardly make sense unless you have a gallery where you have thumbnails and a zoom-in picture. I would add a "highRes" prop to enable this behavior instead.

A good article about this https://blog.twitter.com/engineering/en_us/topics/infrastructure/2019/capping-image-fidelity-on-ultra-high-resolution-devices.html

Suggested change

const DEFAULT_PIXEL_DENSITIES = [0.25, 0.5, 1, 2, 3]

const DEFAULT_PIXEL_DENSITIES = [0.25, 0.5, 1, 2]

Rather than a highRes prop, we let the user pass in an array of densities. Agreed that the default should be a max of 2 though

highRes is food for thought cause it might be easier for people to understand that than srcBreakPoints. Again, not for now ;)

wardpeet · 2020-10-20T09:31:06Z

packages/gatsby-plugin-sharp/src/utils.js

+}
+
+const DEFAULT_PIXEL_DENSITIES = [0.25, 0.5, 1, 2, 3]
+const DEFAULT_FLUID_SIZE = 800
 export const calculateImageSizes = ({


I would split up this function where fixed & fluid is calculated so it's easier to read

if (layout === 'fixed') { sizes = calculateFixedSizes(...) } elseif (layout === 'fluid' || layout === 'constrained') { sizes = calculatedFluidSizes() }

wardpeet · 2020-10-20T09:38:04Z

packages/gatsby-plugin-sharp/src/utils.js

-    // FIXED
-    // if no width is passed, we need to resize the image based on the passed height
-    const fixedDimension = width === undefined ? `height` : `width`
+    checkIgnoredParameters(`fixed`, { maxWidth, maxHeight }, file.absolutePath)


I would call this function differently, it's doing more than checking. TBH I don't have a good name for it 😂

Suggested change

checkIgnoredParameters(`fixed`, { maxWidth, maxHeight }, file.absolutePath)

warnIgnoredParametersUsage(`fixed`, { maxWidth, maxHeight }, file.absolutePath)

wardpeet · 2020-10-20T09:39:45Z

packages/gatsby-plugin-sharp/src/utils.js

    }

    // Create sizes (in width) for the image if no custom breakpoints are
    // provided. If the max width of the container for the rendered markdown file
-    // is 800px, the sizes would then be: 200, 400, 800, 1200, 1600.
+    // is 800px, the sizes would then be: 200, 400, 800, 1600, 2400 (based off of


When we drop 3x

Suggested change

// is 800px, the sizes would then be: 200, 400, 800, 1600, 2400 (based off of

// is 800px, the sizes would then be: 200, 400, 800, 1600 (based off of

ascorbic · 2020-10-20T10:45:28Z

I've done Ward's suggestions so we can get this in.

@wardpeet

* Add static image plugin * Parse static image files * Fix package.json * Better static analysis * Use jsx utils package * Fix typings * Typecheck fix * Fix repo fields * Helpful warning * Re-enable duotone * Update readme * Improve typings * wip fixed image fields for tracedSVG and webP * handle fluid images as well * Use require rather than JSON import * Watch files for changes, and use relative paths * Improve types * Add type * Update yarn.lock * Add lots of comments and remove unused stuff * Create and watch our own nodes * Update readme * Update deps * Only watch in develop * Rename to gatsby-plugin-image * Rearrange, ready for merger * Import @wardpeet 's gatsby-image-netxtgen * Update typings and further merge packages * Build babel plugin * More merging * Add server/browser static image variants * include webpack changes * change error message to use gatsby-plugin-image * Fix compat compilation * Fix SSR * Merge readmes * Build browser bundle * Export correctly from browser entry point * Remove hook import * Apply suggestions from code review Co-authored-by: LB <laurie@gatsbyjs.com> * Changes from review * add path prefix, static image do e2e test * Add prepare script * Apply README suggestions from code review Co-authored-by: LB <laurie@gatsbyjs.com> * Fix image component in e2e test * Better error messages * Prefer svg over base64. Warn for unsupported art direction * Fix type * Use "layout" instead of fixed/fluid * add static images to production runtime site * add path prefix tests * Reorg types. Change private prop name * initial gatsby static image tests * add test suite to circle ci * add test suite to circle ci * add test suite to circle ci * remove unused imports, use valid fluid setting * Use new fluid props syntax * Better var name * update tests * remove tests that no longer match the expected DOM * More compat-fixes * Change classname to match old version * Compat improvements * Update tests to match new classname * v0.0.1 * Add readme caveat * Update version in packages * Remove forcewrapper * wip * have static image use gatsbyImageProps function * Upgrade sharp * Fix static image type * First steps to update API * Add descriptions * TypeScript fixes * Compile ts * Add @gillkyle's calculateImageSizes util funciton * remove unused file * unbump sharp until sharp PR is merged * comment out import * update snapshots * change JSX to take image prop * static image uses new image props * Rename all the things * Turns out it wasn't all of them * Update schema * fix intrinsic issue 1px problem, still doesn't seem to reach the correct max size * Fix layout * Update readme * Fix dep * Shouldn;t be in this * Fix sharp function name * Version * What is this snapshot out of date? * Add a helper funciton * Make backgroundColor work * update layout for images * no longer using base64 so remove that test * placeholder not getting passed from static, so this is still busted * Fix passing of props * Add TODO comments * Export batch queue function * Handle the default sizes elsewhere, as it depends on layout * feat(gatsby-plugin-sharp): create image sizes helper (#27554) * wip tests and tweaks to utility * more wip tests * remove file reading from helper * fixing fluid calculations and adding more tests * tweak a little more * remove extraneous export * check that user specified dimensions are positive * remove unnecessary code from review suggestions * add warning, simplify case of no fluid dimensions * add files to warnings and clean up logic * Split into two functions and remove 3x default * Rename warn function and use reporter Co-authored-by: Matt Kane <matt@gatsbyjs.com> * Use passed-in reporter * Use shared function to handle fit calculation * Return presentationW/H and aspect ratio from sizes function * Round sizes * Use new resizing functions * default layout wasn't passed to createImageSizes * add fixed width default and round calculated height * reversed logic for aspect ratio calculations * Correct description for outputPixelDensities resolver * Remove log * Handle null image in SSR * Version fix * fix(gatsby-plugin-utils): skip libcheck during typegen * Remove unused func * Remove dominantColor from fixed and fluid * sizing code for KG to drive refactor * remove isSmaller * fix tests * Fix sizes and srcset generation * Add object-fit * rename placeholder function for consistency * Update readme * fix path prefix images * fix sizing when larger image dimensions are requested, fix warning as well * rename file to match constrained naming * Changes from review * Add alpha warning * Change from review * fix placeholder opacity * fix placeholder bug * only change placeholder size for background color div * tests: Add visual regression tests (#27619) * tests: Add visual regression tests * Add config * Wait for images to load * Do multiple window sizes * Adjust settings for comparisons * Fix wrapper * Add 1024 window * Save reports * Update fixed image too large snapshot * Disable video * Update readme * Update element id * Force 1x pixel density * Update readme * Fix e2e test * dont flash placeholder image when removing opacity * Fix dependency * add analagous tests for static image * Add width to constrained images and update snapshots * Use float Co-authored-by: Kyle Gill <kylerobertgill@gmail.com> Co-authored-by: Laurie <laurie@gatsbyjs.com> Co-authored-by: gatsbybot <mathews.kyle+gatsbybot@gmail.com>

@wardpeet

* Add static image plugin * Parse static image files * Fix package.json * Better static analysis * Use jsx utils package * Fix typings * Typecheck fix * Fix repo fields * Helpful warning * Re-enable duotone * Update readme * Improve typings * wip fixed image fields for tracedSVG and webP * handle fluid images as well * Use require rather than JSON import * Watch files for changes, and use relative paths * Improve types * Add type * Update yarn.lock * Add lots of comments and remove unused stuff * Create and watch our own nodes * Update readme * Update deps * Only watch in develop * Rename to gatsby-plugin-image * Rearrange, ready for merger * Import @wardpeet 's gatsby-image-netxtgen * Update typings and further merge packages * Build babel plugin * More merging * Add server/browser static image variants * include webpack changes * change error message to use gatsby-plugin-image * Fix compat compilation * Fix SSR * Merge readmes * Build browser bundle * Export correctly from browser entry point * Remove hook import * Apply suggestions from code review Co-authored-by: LB <laurie@gatsbyjs.com> * Changes from review * add path prefix, static image do e2e test * Add prepare script * Apply README suggestions from code review Co-authored-by: LB <laurie@gatsbyjs.com> * Fix image component in e2e test * Better error messages * Prefer svg over base64. Warn for unsupported art direction * Fix type * Use "layout" instead of fixed/fluid * add static images to production runtime site * add path prefix tests * Reorg types. Change private prop name * initial gatsby static image tests * add test suite to circle ci * add test suite to circle ci * add test suite to circle ci * remove unused imports, use valid fluid setting * Use new fluid props syntax * Better var name * update tests * remove tests that no longer match the expected DOM * More compat-fixes * Change classname to match old version * Compat improvements * Update tests to match new classname * v0.0.1 * Add readme caveat * Update version in packages * Remove forcewrapper * wip * have static image use gatsbyImageProps function * Upgrade sharp * Fix static image type * First steps to update API * Add descriptions * TypeScript fixes * Compile ts * Add @gillkyle's calculateImageSizes util funciton * remove unused file * unbump sharp until sharp PR is merged * comment out import * update snapshots * change JSX to take image prop * static image uses new image props * Rename all the things * Turns out it wasn't all of them * Update schema * fix intrinsic issue 1px problem, still doesn't seem to reach the correct max size * Fix layout * Update readme * Fix dep * Shouldn;t be in this * Fix sharp function name * Version * What is this snapshot out of date? * Add a helper funciton * Make backgroundColor work * update layout for images * no longer using base64 so remove that test * placeholder not getting passed from static, so this is still busted * Fix passing of props * Add TODO comments * Export batch queue function * Handle the default sizes elsewhere, as it depends on layout * feat(gatsby-plugin-sharp): create image sizes helper (#27554) * wip tests and tweaks to utility * more wip tests * remove file reading from helper * fixing fluid calculations and adding more tests * tweak a little more * remove extraneous export * check that user specified dimensions are positive * remove unnecessary code from review suggestions * add warning, simplify case of no fluid dimensions * add files to warnings and clean up logic * Split into two functions and remove 3x default * Rename warn function and use reporter Co-authored-by: Matt Kane <matt@gatsbyjs.com> * Use passed-in reporter * Use shared function to handle fit calculation * Return presentationW/H and aspect ratio from sizes function * Round sizes * Use new resizing functions * default layout wasn't passed to createImageSizes * add fixed width default and round calculated height * reversed logic for aspect ratio calculations * Correct description for outputPixelDensities resolver * Remove log * Handle null image in SSR * Version fix * fix(gatsby-plugin-utils): skip libcheck during typegen * Remove unused func * Remove dominantColor from fixed and fluid * sizing code for KG to drive refactor * remove isSmaller * fix tests * Fix sizes and srcset generation * Add object-fit * rename placeholder function for consistency * Update readme * fix path prefix images * fix sizing when larger image dimensions are requested, fix warning as well * rename file to match constrained naming * Changes from review * Add alpha warning * Change from review * fix placeholder opacity * fix placeholder bug * only change placeholder size for background color div * tests: Add visual regression tests (#27619) * tests: Add visual regression tests * Add config * Wait for images to load * Do multiple window sizes * Adjust settings for comparisons * Fix wrapper * Add 1024 window * Save reports * Update fixed image too large snapshot * Disable video * Update readme * Update element id * Force 1x pixel density * Update readme * Fix e2e test * dont flash placeholder image when removing opacity * Fix dependency * Continue moving options into options objects * Update API * Default to blurred * Fix test * Fix default * Change from review * Updates to fixed sizing * Update snapshots * fix readme, clean up unused variable * fix lint * weird readme lint issue Co-authored-by: Kyle Gill <kylerobertgill@gmail.com> Co-authored-by: Laurie <laurie@gatsbyjs.com> Co-authored-by: gatsbybot <mathews.kyle+gatsbybot@gmail.com>

gillkyle added 5 commits October 15, 2020 16:30

wip tests and tweaks to utility

c6c8c06

more wip tests

c4c2a42

remove file reading from helper

a68bf27

fixing fluid calculations and adding more tests

6ec92e5

tweak a little more

1b3d064

gillkyle requested a review from ascorbic October 19, 2020 15:21

gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Oct 19, 2020

laurieontech reviewed Oct 19, 2020

View reviewed changes

packages/gatsby-plugin-sharp/src/utils.js Outdated Show resolved Hide resolved

ascorbic reviewed Oct 19, 2020

View reviewed changes

ascorbic changed the title ~~chore(gatsby-plugin-sharp): create image sizes helper~~ feat(gatsby-plugin-sharp): create image sizes helper Oct 19, 2020

gillkyle added 2 commits October 19, 2020 10:29

remove extraneous export

514ac84

check that user specified dimensions are positive

f90f4bf

gillkyle added 3 commits October 19, 2020 11:15

remove unnecessary code from review suggestions

35b6ccf

add warning, simplify case of no fluid dimensions

c399322

add files to warnings and clean up logic

e421570

gillkyle requested a review from ascorbic October 19, 2020 21:24

ascorbic approved these changes Oct 20, 2020

View reviewed changes

Merge branch 'feat/image-resolvers' into image-sizes-helper

0dff1b9

wardpeet suggested changes Oct 20, 2020

View reviewed changes

ascorbic added 2 commits October 20, 2020 11:39

Split into two functions and remove 3x default

15fb9e2

Rename warn function and use reporter

b399826

ascorbic merged commit 99d5c77 into feat/image-resolvers Oct 20, 2020

delete-merged-branch bot deleted the image-sizes-helper branch October 20, 2020 10:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(gatsby-plugin-sharp): create image sizes helper #27554

feat(gatsby-plugin-sharp): create image sizes helper #27554

gillkyle commented Oct 19, 2020

ascorbic Oct 19, 2020

gillkyle Oct 19, 2020

ascorbic Oct 20, 2020

ascorbic Oct 19, 2020

gillkyle Oct 19, 2020

gillkyle Oct 19, 2020

ascorbic Oct 19, 2020

gillkyle Oct 19, 2020

ascorbic Oct 19, 2020

laurieontech Oct 19, 2020

gillkyle Oct 19, 2020

gillkyle Oct 19, 2020

wardpeet left a comment

wardpeet Oct 20, 2020

wardpeet Oct 20, 2020

ascorbic Oct 20, 2020

wardpeet Oct 20, 2020

wardpeet Oct 20, 2020

wardpeet Oct 20, 2020 •

edited

Loading

wardpeet Oct 20, 2020

ascorbic commented Oct 20, 2020

	const DEFAULT_PIXEL_DENSITIES = [0.25, 0.5, 1, 2, 3]
	const DEFAULT_PIXEL_DENSITIES = [0.25, 0.5, 1, 2]

	checkIgnoredParameters(`fixed`, { maxWidth, maxHeight }, file.absolutePath)
	warnIgnoredParametersUsage(`fixed`, { maxWidth, maxHeight }, file.absolutePath)

	// is 800px, the sizes would then be: 200, 400, 800, 1600, 2400 (based off of
	// is 800px, the sizes would then be: 200, 400, 800, 1600 (based off of

feat(gatsby-plugin-sharp): create image sizes helper #27554

feat(gatsby-plugin-sharp): create image sizes helper #27554

Conversation

gillkyle commented Oct 19, 2020

Description

Related Issues

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wardpeet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wardpeet Oct 20, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ascorbic commented Oct 20, 2020

wardpeet Oct 20, 2020 •

edited

Loading