feat: Add new image resolvers #27443

ascorbic · 2020-10-14T12:18:52Z

This PR adds a new GraphQL API for images, for use with the new gatsby-plugin-image. It adds a new resolver to the ImageSharp node, with a single field imageData. Unlike the existing fixed and fluid resolvers, this returns a JSON type, meaning you don't specify the individual fields, but are instead given the whole object. This is because the object is then passed in to the <GatsbyImage> component. The API is like this:

coverImage: file(relativePath: { eq: "plant.jpg" }) {
  childImageSharp {
    gatsbyImage(maxWidth: 720, layout: FLUID, placeholder: TRACED_SVG) {
      imageData
    }
  }
}

import { GatsbyImage, getImage } from "gatsby-plugin-image"

export function Plant({ data }) {
    const imageData = getImage(data.coverImage)
    return <GatsbyImage image={imageData} />
}

The helper function getImage takes a file node and returns file?.childImageSharp?.gatsbyImage?.imageData

Because this no longer uses fragments to specify which fields to return, it instead uses arguments passed to the resolver. These include:

placeholder: Format of generated placeholder image.
DOMINANT_COLOR: a solid color, calculated from the dominant color of the image. (default) Currently disabled until sharp is updated
BLURRED: a blurred, low resolution image, encoded as a base64 data URI
TRACED_SVG: a low-resolution traced SVG of the image.
NONE: no placeholder. Set "background" to use a fixed background color.
layout: The layout for the image.
FIXED: A static image sized, that does not resize according to the screen width
FLUID: The image resizes to fit its container. Pass a "sizes" option if it isn't going to be the full width of the screen.
CONSTRAINED: Resizes to fit its container, up to a maximum width, at which point it will remain fixed in size.
outputPixelDensities: A list of image pixel densities to generate, for high-resolution (retina) screens. It will never generate images larger than the source, and will always a 1x image.
Default is [ 0.25, 0.5, 1, 2 ], for fluid/constrained images, and [ 1, 2 ] for fixed. In this case, an image with a fluid layout and maxWidth = 400 would generate images at 100, 200, 400 and 800px wide
sizes: The "sizes" property, passed to the img tag. This describes the display size of the image.
This does not affect the generated images, but is used by the browser to decide which images to download. You can leave this blank for fixed images, or if the responsive image
container will be the full width of the screen. In these cases we will generate an appropriate value.

This PR now also uses the same functions for static images, so they have a similar API.

[ch16945]
[ch16946]

…-babel

…abel

packages/gatsby-plugin-sharp/src/image-data.ts

wardpeet

I've added some api questions. I don't think we need all options that we currently have in fixed & fluid. We can always add them later.

wardpeet · 2020-10-26T10:11:13Z

packages/gatsby-transformer-sharp/src/customize-schema.js

+        This only applies when layout = FLUID or CONSTRAINED. For other layout types, use "width"`,
+      },
+      maxHeight: {
+        type: GraphQLInt,


should we add description here too?

wardpeet · 2020-10-26T10:11:27Z

packages/gatsby-transformer-sharp/src/customize-schema.js

+        `,
+      },
+      height: {
+        type: GraphQLInt,


packages/gatsby-transformer-sharp/src/customize-schema.js

wardpeet · 2020-10-26T10:14:11Z

packages/gatsby-transformer-sharp/src/customize-schema.js

+      webP: {
+        type: GraphQLBoolean,
+        defaultValue: true,
+        description: `Generate images in WebP format as well as matching the input format. This is the default (and strongly recommended), but will add to processing time.`,
+      },


Any reason why we want this option? :) I don't immediately see the need to disable it.

It doubles the processing time, so some sites where that's a big problem might want to

(Or maybe not doubles, but increases signficantly)

The biggest reason why I wouldn't want people to disable it in here is that they value DX over UX. With lazy images and query-on-demand, this becomes less of an issue IMO but I do get your point.

What if we change the description to talk more about performance and not the built time.

description: `Generate images in WebP format as well as matching the input format. This is the default (and strongly recommended), it improves your website performance.`,

What if the default was false during gatsby develop (i.e. NODE_ENV === development) for better DX perf, but true during gatsby build (i.e. NODE_ENV === production) for better UX?

we'll fix this with lazy images #27603

Build time is still important in production. My last job had a site that wouldn't build within the Netlify time limit if we had WebP enabled. We moved it to AWS, but it was still something we needed to be able to disable.

My plan for the new API will be to allow the user to pass an array of formats, with the default being [AUTO, WEBP]

Build times are never more important than users. That's why I want this description to be different.

wardpeet · 2020-10-26T10:26:51Z

packages/gatsby-transformer-sharp/src/customize-schema.js

+      base64Width: {
+        type: GraphQLInt,
+      },


Can we do the same as tracedSVGOptions?

What do you mean? Combine all base64 options into a single object?

Yes so it's all in one option, I know we only have base64Width but than it's on par with tracedSVG

wardpeet · 2020-10-26T10:29:02Z

packages/gatsby-transformer-sharp/src/customize-schema.js

+      jpegProgressive: {
+        type: GraphQLBoolean,
+        defaultValue: true,
+      },


Unsure if we need this option

Yeah, I just copied it from the current options

wardpeet · 2020-10-26T10:32:12Z

packages/gatsby-transformer-sharp/src/customize-schema.js

+      quality: {
+        type: GraphQLInt,
+      },
+      jpegQuality: {
+        type: GraphQLInt,
+      },
+      pngQuality: {
+        type: GraphQLInt,
+      },
+      webpQuality: {
+        type: GraphQLInt,
+      },


Should we move these things into another inputType so it doesn't add up to args?

You mean create an options object for each format?

wardpeet · 2020-10-26T10:33:25Z

packages/gatsby-transformer-sharp/src/customize-schema.js

+      toFormatBase64: {
+        type: ImageFormatType,
+        defaultValue: ``,
+        description: `Force output format. Default is to use the same as the input format`,
+      },


Unsure if this makes sense into gatsbyProps

Yeah, maybe in an options object for the blurred preview

wardpeet · 2020-10-26T10:34:45Z

packages/gatsby-transformer-sharp/src/customize-schema.js

+      cropFocus: {
+        type: ImageCropFocusType,
+        defaultValue: sharp.strategy.attention,
+      },
+      fit: {
+        type: ImageFitType,
+        defaultValue: sharp.fit.cover,
+      },
+      background: {
+        type: GraphQLString,
+        defaultValue: `rgba(0,0,0,1)`,
+      },
+      rotate: {
+        type: GraphQLInt,
+        defaultValue: 0,
+      },
+      trim: {
+        type: GraphQLFloat,
+        defaultValue: false,
+      },


Do we want to put this into an inputType as well?

wardpeet · 2020-10-26T10:36:29Z

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

-const ProgressBar = require(`progress`)
-
+import ProgressBar from "progress"
+import sharp from "sharp"


Please use

Suggested change

import sharp from "sharp"

import sharp from "./safe-sharp"

ascorbic · 2020-10-26T10:57:20Z

@wardpeet @laurieontech @gillkyle I'm going to create a doc with all the options and we can discuss them in more depth. I think it does make sense to put some time into getting this right.

ascorbic · 2020-10-26T16:41:48Z

Here is the RFC with the new options proposal: #27668

* 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

…o feat/image-resolvers

wardpeet

I don't have anything really blocking. Some small nits and follow-ups but nothing to block this PR from being merged as long as we fix those follow ups.

Visual regression test is failing for me locally;

The visual regression test is a great addition! Thank you!

What's next?

We'll need e2e test for disabled javascript and when nativeImageLazyLoading is not available.
test if eager is actually working as expected
move gatsby-static-image to gatsby-image e2e test
Test gatsbyImage resolver in the e2e tests

wardpeet · 2020-10-29T09:36:22Z

packages/gatsby-plugin-image/src/components/hooks.ts

+    "data-placeholder-image": ``,
+    style: {
+      opacity: isLoaded ? 0 : 1,
+      transition: `opacity 500ms linear`,


Shouldn't this be 250 like the main image?

Suggested change

transition: `opacity 500ms linear`,

transition: `opacity 250ms linear`,

@gillkyle Could you look at this?

Since the placeholder needs to fade out, I made the placholder 500ms so you won't have a really gradual fade in from the empty space as the main image fades in (in 250ms). You can see what this looks like in the current state vs the suggested:

250ms for placeholder, 500ms for main 500ms for placeholder, 500ms for main image

@wardpeet

wardpeet · 2020-10-29T09:38:46Z

packages/gatsby-plugin-image/src/components/hooks.ts

  }

  return result
 }

 export type PlaceholderImageAttrs = ImgHTMLAttributes<HTMLImageElement> &
-  Pick<PlaceholderProps, "sources" | "fallback">
+  Pick<PlaceholderProps, "sources" | "fallback"> & {
+    "data-placeholder-image"?: string


Follow up:
Let's move to data-gatsby-image-placeholder, let's do the same with main image

wardpeet · 2020-10-29T09:40:10Z

packages/gatsby-plugin-image/src/components/placeholder.tsx

 export const Placeholder: FunctionComponent<PlaceholderProps> = function Placeholder({
  fallback,
  ...props
 }) {
-  return (
-    <Picture
-      {...props}
-      fallback={{
-        src: fallback,
-      }}
-      aria-hidden
-      alt=""
-    />
-  )
+  if (fallback) {
+    return (
+      <Picture
+        {...props}
+        fallback={{
+          src: fallback,
+        }}
+        aria-hidden
+        alt=""
+      />
+    )
+  } else {
+    return <div {...props}></div>
+  }
 }


Follow up:
this is going to cause issue with composability when we want devs to create their own version. Can we add a background prop? This allows us to update proptypes to be fallback or background

wardpeet · 2020-10-29T09:45:46Z

packages/gatsby-plugin-image/src/components/gatsby-image.browser.tsx

+  if (!image) {
+    if (process.env.NODE_ENV === `development`) {
+      console.warn(`[gatsby-plugin-image] Missing image prop`)
+    }
+    return null
+  }


Follow up:
Shouldn't this be handled by propTypes?

wardpeet · 2020-10-29T09:47:05Z

packages/gatsby-plugin-image/src/gatsby-ssr.tsx

+          target.style.opacity = 1;
+          if (placeholder) {
+            placeholder.style.opacity = 0;
+            placeholder.style.transition = "opacity 500ms linear";


should this be 250?

Suggested change

placeholder.style.transition = "opacity 500ms linear";

placeholder.style.transition = "opacity 250ms linear";

wardpeet · 2020-10-29T10:00:53Z

packages/gatsby-plugin-sharp/src/image-data.ts

+
+  const primaryImage = images[primaryIndex]
+
+  if (!images?.length) {


images should always be a [] unsure why ou need the ?

Suggested change

if (!images?.length) {

if (!images.length) {

wardpeet · 2020-10-29T10:01:21Z

packages/gatsby-plugin-sharp/src/image-data.ts

+    const transforms = imageSizes.sizes.map(outputWidth => {
+      const width = Math.round(outputWidth)
+      const transform = createTransformObject({
+        ...args,
+        width,
+        height: Math.round(width / imageSizes.aspectRatio),
+        toFormat: `webp`,
+      })
+      return transform
+    })
+
+    const webpImages = batchQueueImageResizing({
+      file,
+      transforms,
+      reporter,
+    })
+    const webpSrcSet = getSrcSet(webpImages)
+
+    imageProps.images.sources?.push({
+      srcSet: webpSrcSet,
+      type: `image/webp`,
+      sizes,
+    })


Follow up:
Any reason why we don't append webp to the previous batch? Wouldn't that speed up things? (less active jobs)

Or isn't it possible?

Yeah, it should be possible. I'm goign to handle it differently with the updated API

wardpeet · 2020-10-29T10:05:23Z

packages/gatsby-transformer-sharp/src/customize-schema.js

+      webP: {
+        type: GraphQLBoolean,
+        defaultValue: true,
+        description: `Generate images in WebP format as well as matching the input format. This is the default (and strongly recommended), but will add to processing time.`,
+      },


we'll fix this with lazy images #27603

wardpeet · 2020-10-29T10:06:15Z

packages/gatsby-transformer-sharp/src/customize-schema.js

+      outputPixelDensities: {
+        type: GraphQLList(GraphQLInt),
+        description: stripIndent`
+        A list of image pixel densities to generate, for high-resolution (retina) screens. It will never generate images larger than the source, and will always a 1x image. 
+        Default is [ 1, 2 ] for fixed images, meaning 1x, 2x, 3x, and [0.25, 0.5, 1, 2] for fluid. In this case, an image with a fluid layout and width = 400 would generate images at 100, 200, 400 and 800px wide`,
+      },


For fluid this isn't working, this needs to be GraphQLFloat

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

ascorbic and others added 30 commits September 21, 2020 17:15

Add static image plugin

4814d60

Parse static image files

5509d90

Fix package.json

6585d84

Better static analysis

83b0321

Use jsx utils package

b430e5a

Fix typings

96df1b8

Typecheck fix

f98f56a

Fix repo fields

e48da0a

Helpful warning

c11c2a4

Re-enable duotone

7f16e80

Update readme

146bd6e

Improve typings

f46f3e0

Merge branch 'master' into feat/static-image-babel

65d06a4

wip fixed image fields for tracedSVG and webP

45a223f

handle fluid images as well

836fe1d

Use require rather than JSON import

97a5b77

Watch files for changes, and use relative paths

46e5807

Merge branch 'add-missing-static-image-fields' into feat/static-image…

0d66f40

…-babel

Improve types

5e41c30

Add type

0ebbf06

Merge branch 'master' into feat/static-image-babel

41a3cce

Update yarn.lock

12597fa

Add lots of comments and remove unused stuff

4036bb3

Create and watch our own nodes

b548429

Update readme

3242599

Merge remote-tracking branch 'origin/master' into feat/static-image-b…

935e91a

…abel

Update deps

70616e2

Only watch in develop

67ea9ef

Rename to gatsby-plugin-image

4f68363

Rearrange, ready for merger

f5dc9f7

rename file to match constrained naming

f3e114a

laurieontech reviewed Oct 23, 2020

View reviewed changes

packages/gatsby-plugin-sharp/src/image-data.ts Outdated Show resolved Hide resolved

ascorbic added 2 commits October 26, 2020 10:20

Changes from review

b5c546a

Merge remote-tracking branch 'origin' into feat/image-resolvers

89764a8

wardpeet reviewed Oct 26, 2020

View reviewed changes

ascorbic added 2 commits October 26, 2020 15:23

Add alpha warning

950b131

Change from review

2568f00

Laurie and others added 13 commits October 26, 2020 17:35

fix placeholder opacity

43bb48b

fix placeholder bug

fb616c3

only change placeholder size for background color div

3508808

Fix e2e test

9c3b7cb

Merge branch 'feat/image-resolvers' of github.com:gatsbyjs/gatsby int…

d0ff2c8

…o feat/image-resolvers

Merge remote-tracking branch 'origin' into feat/image-resolvers

f9e36bb

Merge remote-tracking branch 'origin' into feat/image-resolvers

4cfc7c3

dont flash placeholder image when removing opacity

de6c6a1

Merge branch 'feat/image-resolvers' of github.com:gatsbyjs/gatsby int…

b0c27ec

…o feat/image-resolvers

Fix dependency

d8675ce

add analagous tests for static image

96118ca

Add width to constrained images and update snapshots

a33d4f6

wardpeet previously approved these changes Oct 29, 2020

View reviewed changes

wardpeet changed the title ~~Add new image resolvers~~ feat: Add new image resolvers Oct 29, 2020

Use float

b4a8e60

ascorbic dismissed wardpeet’s stale review via b4a8e60 October 29, 2020 11:20

ascorbic merged commit e2022c2 into master Oct 29, 2020

delete-merged-branch bot deleted the feat/image-resolvers branch October 29, 2020 11:58

ascorbic restored the feat/image-resolvers branch November 4, 2020 16:28

LekoArts deleted the feat/image-resolvers branch November 9, 2020 08:30

	transition: `opacity 500ms linear`,
	transition: `opacity 250ms linear`,

	placeholder.style.transition = "opacity 500ms linear";
	placeholder.style.transition = "opacity 250ms linear";


		const primaryImage = images[primaryIndex]

		if (!images?.length) {

feat: Add new image resolvers #27443

feat: Add new image resolvers #27443

Conversation

ascorbic commented Oct 14, 2020 • edited Loading

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

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

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

ascorbic commented Oct 26, 2020

ascorbic commented Oct 26, 2020

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

gillkyle Oct 29, 2020 • edited Loading

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

ascorbic commented Oct 14, 2020 •

edited

Loading

gillkyle Oct 29, 2020 •

edited

Loading