fix(gatsby-plugin-image): Fix handling of sizes prop in SSR (#28835)

* fix(gatsby-plugin-image): Fix handling of sizes prop in SSR * Update tests * Update tests * Fix types * Update e2e tests * Update e2e-tests/gatsby-static-image/src/pages/constrained.js Co-authored-by: LB <[email protected]> * Add test for size override Co-authored-by: gatsbybot <[email protected]> Co-authored-by: LB <[email protected]> (cherry picked from commit a135c50)
gatsbyjs · Jan 5, 2021 · ee97904 · ee97904
1 parent afac774
commit ee97904
Show file tree

Hide file tree

Showing 8 changed files with 77 additions and 31 deletions.
diff --git a/e2e-tests/gatsby-static-image/cypress/integration/constrained.js b/e2e-tests/gatsby-static-image/cypress/integration/constrained.js
@@ -0,0 +1,33 @@
+describe(`constrained`, () => {
+  beforeEach(() => {
+    cy.visit(`/constrained`).waitForRouteChange()
+  })
+
+  it(`renders a spacer svg`, () => {
+    cy.getTestElement(`image-constrained`)
+      .find(`img[role=presentation]`)
+      .should(`have.attr`, `src`)
+      .and(`match`, /svg\+xml/)
+  })
+
+  it(`includes sizes attribute with default width`, () => {
+    cy.getTestElement(`image-constrained`)
+      .find(`[data-main-image]`)
+      .should(`have.attr`, `sizes`)
+      .and(`equal`, `(min-width: 4015px) 4015px, 100vw`)
+  })
+
+  it(`includes sizes attribute with specified width`, () => {
+    cy.getTestElement(`image-constrained-limit`)
+      .find(`[data-main-image]`)
+      .should(`have.attr`, `sizes`)
+      .and(`equal`, `(min-width: 500px) 500px, 100vw`)
+  })
+
+  it(`overrides sizes`, () => {
+    cy.getTestElement(`image-constrained-override`)
+      .find(`[data-main-image]`)
+      .should(`have.attr`, `sizes`)
+      .and(`equal`, `100vw`)
+  })
+})
diff --git a/e2e-tests/gatsby-static-image/cypress/integration/fixed.js b/e2e-tests/gatsby-static-image/cypress/integration/fixed.js
@@ -28,4 +28,10 @@ describe(`fixed`, () => {
       .should(`exist`)
   })
 
+  it(`includes sizes attribute`, () => {
+    cy.getTestElement(fixedTestId)
+      .find(`[data-main-image]`)
+      .should(`have.attr`, `sizes`)
+      .should(`equal`, `500px`)
+  })
 })
diff --git a/e2e-tests/gatsby-static-image/cypress/integration/fluid.js b/e2e-tests/gatsby-static-image/cypress/integration/fluid.js
@@ -23,4 +23,11 @@ describe(`fluid`, () => {
       .should(`have.attr`, `style`)
       .and(`match`, /padding/)
   })
+
+  it(`includes sizes attribute`, () => {
+    cy.getTestElement(fluidTestId)
+      .find(`[data-main-image]`)
+      .should(`have.attr`, `sizes`)
+      .should(`equal`, `100vw`)
+  })
 })
diff --git a/e2e-tests/gatsby-static-image/src/pages/constrained.js b/e2e-tests/gatsby-static-image/src/pages/constrained.js
@@ -3,21 +3,27 @@ import { StaticImage } from "gatsby-plugin-image"
 
 import Layout from "../components/layout"
 
-const FluidPage = () => (
+const ConstrainedPage = () => (
   <Layout>
-    <div data-testid="image-fluid">
+    <div data-testid="image-constrained">
       <StaticImage src="../images/citrus-fruits.jpg" alt="Citrus fruits" />
     </div>
-    <div data-testid="image-fluid-png">
-      <StaticImage src="../images/gatsby-icon.png" alt="Gatsby icon" />
+    <div data-testid="image-constrained-limit">
+      <StaticImage
+        src="../images/citrus-fruits.jpg"
+        maxWidth={500}
+        alt="Citrus fruits"
+      />
     </div>
-    <div data-testid="image-fluid-relative">
-      <StaticImage src="../../content/relative.jpg" alt="Citrus fruits" />
-    </div>
-    <div data-testid="invalid-image">
-      <StaticImage src="./does-not-exist.jpg" />
+    <div data-testid="image-constrained-override">
+      <StaticImage
+        src="../images/citrus-fruits.jpg"
+        maxWidth={500}
+        sizes="100vw"
+        alt="Citrus fruits"
+      />
     </div>
   </Layout>
 )
 
-export default FluidPage
+export default ConstrainedPage
diff --git a/packages/gatsby-plugin-image/src/components/__tests__/gatsby-image.browser.tsx b/packages/gatsby-plugin-image/src/components/__tests__/gatsby-image.browser.tsx
@@ -1,5 +1,5 @@
 import React from "react"
-import { GatsbyImage, ISharpGatsbyImageData } from "../gatsby-image.browser"
+import { GatsbyImage, IGatsbyImageData } from "../gatsby-image.browser"
 import { render, waitFor } from "@testing-library/react"
 import * as hooks from "../hooks"
 
@@ -14,7 +14,7 @@ jest.mock(`../../../macros/terser.macro`, () => (strs): string => strs.join(``))
 
 describe(`GatsbyImage browser`, () => {
   let beforeHydrationContent: HTMLDivElement
-  let image: ISharpGatsbyImageData
+  let image: IGatsbyImageData
 
   beforeEach(() => {
     console.warn = jest.fn()
@@ -27,9 +27,9 @@ describe(`GatsbyImage browser`, () => {
       width: 100,
       height: 100,
       layout: `fluid`,
-      images: { fallback: { src: `some-src-fallback.jpg` } },
+      images: { fallback: { src: `some-src-fallback.jpg`, sizes: `192x192` } },
       placeholder: { sources: [] },
-      sizes: `192x192`,
+
       backgroundColor: `red`,
     }
 
@@ -168,7 +168,7 @@ describe(`GatsbyImage browser`, () => {
     expect(onStartLoadSpy).toBeCalledWith({ wasCached: false })
     expect(onLoadSpy).toBeCalled()
     expect(hooks.storeImageloaded).toBeCalledWith(
-      `{"fallback":{"src":"some-src-fallback.jpg"}}`
+      `{"fallback":{"src":"some-src-fallback.jpg","sizes":"192x192"}}`
     )
   })
 

diff --git a/packages/gatsby-plugin-image/src/components/__tests__/gatsby-image.server.tsx b/packages/gatsby-plugin-image/src/components/__tests__/gatsby-image.server.tsx
@@ -170,7 +170,6 @@ describe(`GatsbyImage server`, () => {
         layout: `constrained`,
         images,
         placeholder: { sources: [] },
-        sizes: `192x192`,
         backgroundColor: `red`,
       }
 
@@ -186,22 +185,22 @@ describe(`GatsbyImage server`, () => {
           data-main-image=""
           decoding="async"
           loading="lazy"
-          sizes="192x192"
           style="opacity: 0;"
         />
       `)
     })
 
     it(`has a valid src value when fallback is provided in images`, () => {
-      const images = { fallback: { src: `some-src-fallback.jpg` } }
+      const images = {
+        fallback: { src: `some-src-fallback.jpg`, sizes: `192x192` },
+      }
 
       const image: IGatsbyImageData = {
         width: 100,
         height: 100,
         layout: `constrained`,
         images,
         placeholder: { sources: [] },
-        sizes: `192x192`,
         backgroundColor: `red`,
       }
 
@@ -233,6 +232,7 @@ icon64px.png 64w,
 icon-retina.png 2x,
 icon-ultra.png 3x,
 icon.svg`,
+          sizes: `192x192`,
         },
       }
 
@@ -242,7 +242,6 @@ icon.svg`,
         layout: `constrained`,
         images,
         placeholder: { sources: [] },
-        sizes: `192x192`,
         backgroundColor: `red`,
       }
 
@@ -278,7 +277,6 @@ icon.svg`,
         layout: `constrained`,
         images,
         placeholder: { sources: [] },
-        sizes: `192x192`,
         backgroundColor: `red`,
       }
 
@@ -294,7 +292,6 @@ icon.svg`,
           data-main-image=""
           decoding="async"
           loading="lazy"
-          sizes="192x192"
           style="opacity: 0;"
         />
       `)
@@ -317,9 +314,11 @@ icon.svg`,
         width: 100,
         height: 100,
         layout: `constrained`,
-        images: { sources },
+        images: {
+          sources,
+          fallback: { src: `some-src-fallback.jpg`, sizes: `192x192` },
+        },
         placeholder: { sources: [] },
-        sizes: `192x192`,
         backgroundColor: `red`,
       }
 
@@ -339,6 +338,7 @@ icon.svg`,
             alt="A fake image for testing purpose"
             data-gatsby-image-ssr=""
             data-main-image=""
+            data-src="some-src-fallback.jpg"
             decoding="async"
             loading="lazy"
             sizes="192x192"
@@ -357,7 +357,6 @@ icon.svg`,
         layout: `fluid`,
         images: {},
         placeholder: { sources: [] },
-        sizes: `192x192`,
         backgroundColor: `red`,
       }
 
@@ -383,7 +382,6 @@ icon.svg`,
         layout: `fixed`,
         images: {},
         placeholder: { sources: [] },
-        sizes: `192x192`,
         backgroundColor: `red`,
       }
 
@@ -409,7 +407,6 @@ icon.svg`,
         layout: `constrained`,
         images: {},
         placeholder: { sources: [] },
-        sizes: `192x192`,
         backgroundColor: `red`,
       }
 

diff --git a/packages/gatsby-plugin-image/src/components/gatsby-image.browser.tsx b/packages/gatsby-plugin-image/src/components/gatsby-image.browser.tsx
@@ -43,7 +43,6 @@ export interface IGatsbyImageData {
   layout: Layout
   height?: number
   backgroundColor?: string
-  sizes?: string
   images: Pick<MainImageProps, "sources" | "fallback">
   placeholder?: Pick<PlaceholderProps, "sources" | "fallback">
   width?: number

diff --git a/packages/gatsby-plugin-image/src/components/gatsby-image.server.tsx b/packages/gatsby-plugin-image/src/components/gatsby-image.server.tsx
@@ -49,7 +49,6 @@ export const GatsbyImage: FunctionComponent<GatsbyImageProps> = function GatsbyI
     layout,
     images,
     placeholder,
-    sizes,
     backgroundColor: placeholderBackgroundColor,
   } = image
 
@@ -65,7 +64,7 @@ export const GatsbyImage: FunctionComponent<GatsbyImageProps> = function GatsbyI
   }
   if (images.fallback) {
     cleanedImages.fallback = {
-      src: images.fallback.src,
+      ...images.fallback,
       srcSet: images.fallback.srcSet
         ? removeNewLines(images.fallback.srcSet)
         : undefined,
@@ -106,7 +105,6 @@ export const GatsbyImage: FunctionComponent<GatsbyImageProps> = function GatsbyI
 
         <MainImage
           data-gatsby-image-ssr=""
-          sizes={sizes}
           className={imgClassName}
           style={imgStyle}
           {...(props as Omit<MainImageProps, "images" | "fallback">)}