Always add height and width prop to image with layout="raw" (#36523)

This PR makes the following changes: * Always add the `height` and `width` prop to image with `raw` layout (previously only added to images without `sizes`) * Add a warning if a raw layout image is getting stretched (which can be caused by interaction of height and width prop with styles) * Remove automatic aspect-ratio style from `raw` images. This is no longer necessary if all `raw` images have height and width props. * Update tests and docs accordingly
vercel · Apr 27, 2022 · 93d8fac · 93d8fac
1 parent af1d7c9
commit 93d8fac
Show file tree

Hide file tree

Showing 4 changed files with 57 additions and 34 deletions.
diff --git a/docs/api-reference/next/image.md b/docs/api-reference/next/image.md
@@ -50,19 +50,19 @@ When using an external URL, you must add it to
 
 The `width` property can represent either the _rendered_ width or _original_ width in pixels, depending on the [`layout`](#layout) and [`sizes`](#sizes) properties.
 
-When using `layout="intrinsic"`, `layout="fixed"`, or `layout="raw"` without `sizes`, the `width` property represents the _rendered_ width in pixels, so it will affect how large the image appears.
+When using `layout="intrinsic"`, `layout="fixed"`, or `layout="raw"`, the `width` property represents the _rendered_ width in pixels, so it will affect how large the image appears.
 
-When using `layout="responsive"`, `layout="fill"`, or `layout="raw"` with `sizes`, the `width` property represents the _original_ width in pixels, so it will only affect the aspect ratio.
+When using `layout="responsive"`, `layout="fill"`, the `width` property represents the _original_ width in pixels, so it will only affect the aspect ratio.
 
 The `width` property is required, except for [statically imported images](#local-images), or those with `layout="fill"`.
 
 ### height
 
 The `height` property can represent either the _rendered_ height or _original_ height in pixels, depending on the [`layout`](#layout) and [`sizes`](#sizes) properties.
 
-When using `layout="intrinsic"`, `layout="fixed"`, or `layout="raw"` without `sizes`, the `height` property represents the _rendered_ height in pixels, so it will affect how large the image appears.
+When using `layout="intrinsic"`, `layout="fixed"`, or `layout="raw"`, the `height` property represents the _rendered_ height in pixels, so it will affect how large the image appears.
 
-When using `layout="responsive"`, `layout="fill"`, or `layout="raw"` with `sizes`, the `height` property represents the _original_ height in pixels, so it will only affect the aspect ratio.
+When using `layout="responsive"`, `layout="fill"`, the `height` property represents the _original_ height in pixels, so it will only affect the aspect ratio.
 
 The `height` property is required, except for [statically imported images](#local-images), or those with `layout="fill"`.
 
@@ -94,8 +94,7 @@ The layout behavior of the image as the viewport changes size.
   - This is usually paired with the [`objectFit`](#objectFit) property.
   - Ensure the parent element has `position: relative` in their stylesheet.
 - When `raw`[\*](#experimental-raw-layout-mode), the image will be rendered as a single image element with no wrappers, sizers or other responsive behavior.
-  - If your image styling will change the size of a `raw` image, you should include the `sizes` property for proper image serving. Otherwise your image will receive a fixed height and width.
-  - The other layout modes are optimized for performance and should cover nearly all use cases. It is recommended to try to use those modes before using `raw`.
+  - If your image styling will change the size of a `raw` image, you should include the `sizes` property for proper image serving. Otherwise your image will be requested as though it has fixed width and height.
 - [Demo background image](https://image-component.nextjs.gallery/background)
 
 ### loader
@@ -181,6 +180,8 @@ Allows [passing CSS styles](https://reactjs.org/docs/dom-elements.html#style) to
 
 Note that all `layout` modes other than `"raw"`[\*](#experimental-raw-layout-mode) apply their own styles to the image element, and these automatic styles take precedence over the `style` prop.
 
+Also keep in mind that the required `width` and `height` props can interact with your styling. If you use styling to modify an image's `width`, you must set the `height="auto"` style as well, or your image will be distorted.
+
 ### objectFit
 
 Defines how the image will fit into its parent container when using `layout="fill"`.
@@ -493,10 +494,6 @@ module.exports = {
 }
 ```
 
-> Note on CLS with `layout="raw"`:
-> It is possible to cause [layout shift](https://web.dev/cls/) with the image component in `raw` mode. If you include a `sizes` property, the image component will not pass `height` and `width` attributes to the image, to allow you to apply your own responsive sizing.
-> An [aspect-ratio](https://developer.mozilla.org/en-US/docs/Web/CSS/aspect-ratio) style property is automatically applied to prevent layout shift, but this won't apply on [older browsers](https://caniuse.com/mdn-css_properties_aspect-ratio).
-
 ### Animated Images
 
 The default [loader](#loader) will automatically bypass Image Optimization for animated images and serve the image as-is.

diff --git a/packages/next/client/image.tsx b/packages/next/client/image.tsx
@@ -329,6 +329,19 @@ function handleLoading(
       onLoadingCompleteRef.current({ naturalWidth, naturalHeight })
     }
     if (process.env.NODE_ENV !== 'production') {
+      if (layout === 'raw') {
+        const heightModified =
+          img.height.toString() !== img.getAttribute('height')
+        const widthModified = img.width.toString() !== img.getAttribute('width')
+        if (
+          (heightModified && !widthModified) ||
+          (!heightModified && widthModified)
+        ) {
+          warnOnce(
+            `Image with src "${src}" has either width or height modified, but not the other. If you use CSS to change the size of your image, also include the styles 'width: "auto"' or 'height: "auto"' to maintain the aspect ratio.`
+          )
+        }
+      }
       if (img.parentElement?.parentElement) {
         const parent = getComputedStyle(img.parentElement.parentElement)
         if (!parent.position) {
@@ -664,13 +677,7 @@ export default function Image({
     }
   }
 
-  const imgStyle = Object.assign(
-    {},
-    style,
-    layout === 'raw'
-      ? { aspectRatio: `${widthInt} / ${heightInt}` }
-      : layoutStyle
-  )
+  const imgStyle = Object.assign({}, style, layout === 'raw' ? {} : layoutStyle)
   const blurStyle =
     placeholder === 'blur' && !blurComplete
       ? {
@@ -893,9 +900,7 @@ const ImageElement = ({
       <img
         {...rest}
         {...imgAttributes}
-        {...(layout === 'raw' && !imgAttributes.sizes
-          ? { height: heightInt, width: widthInt }
-          : {})}
+        {...(layout === 'raw' ? { height: heightInt, width: widthInt } : {})}
         decoding="async"
         data-nimg={layout}
         className={className}
@@ -961,7 +966,7 @@ const ImageElement = ({
               sizes: imgAttributes.sizes,
               loader,
             })}
-            {...(layout === 'raw' && !imgAttributes.sizes
+            {...(layout === 'raw'
               ? { height: heightInt, width: widthInt }
               : {})}
             decoding="async"

diff --git a/test/integration/image-component/default/pages/layout-raw.js b/test/integration/image-component/default/pages/layout-raw.js
@@ -40,6 +40,17 @@ const Page = () => {
           loading="eager"
         ></Image>
       </div>
+      <div id="image-container4">
+        <Image
+          id="raw4"
+          src="/test.png"
+          width="400"
+          height="400"
+          layout="raw"
+          loading="eager"
+          style={{ width: '50%', height: 'auto' }}
+        ></Image>
+      </div>
     </div>
   )
 }

diff --git a/test/integration/image-component/default/test/index.test.js b/test/integration/image-component/default/test/index.test.js
@@ -703,9 +703,7 @@ function runTests(mode) {
       )
       expect(childElementType).toBe('IMG')
 
-      expect(await browser.elementById('raw1').getAttribute('style')).toBe(
-        `aspect-ratio:1200 / 700`
-      )
+      expect(await browser.elementById('raw1').getAttribute('style')).toBeNull()
       expect(await browser.elementById('raw1').getAttribute('height')).toBe(
         '700'
       )
@@ -717,22 +715,34 @@ function runTests(mode) {
       )
 
       expect(await browser.elementById('raw2').getAttribute('style')).toBe(
-        'padding-left:4rem;width:100%;object-position:30% 30%;aspect-ratio:1200 / 700'
+        'padding-left:4rem;width:100%;object-position:30% 30%'
+      )
+      expect(await browser.elementById('raw2').getAttribute('height')).toBe(
+        '700'
+      )
+      expect(await browser.elementById('raw2').getAttribute('width')).toBe(
+        '1200'
       )
-      expect(
-        await browser.elementById('raw2').getAttribute('height')
-      ).toBeNull()
-      expect(await browser.elementById('raw2').getAttribute('width')).toBeNull()
       expect(await browser.elementById('raw2').getAttribute('srcset')).toBe(
         `/_next/image?url=%2Fwide.png&w=16&q=75 16w, /_next/image?url=%2Fwide.png&w=32&q=75 32w, /_next/image?url=%2Fwide.png&w=48&q=75 48w, /_next/image?url=%2Fwide.png&w=64&q=75 64w, /_next/image?url=%2Fwide.png&w=96&q=75 96w, /_next/image?url=%2Fwide.png&w=128&q=75 128w, /_next/image?url=%2Fwide.png&w=256&q=75 256w, /_next/image?url=%2Fwide.png&w=384&q=75 384w, /_next/image?url=%2Fwide.png&w=640&q=75 640w, /_next/image?url=%2Fwide.png&w=750&q=75 750w, /_next/image?url=%2Fwide.png&w=828&q=75 828w, /_next/image?url=%2Fwide.png&w=1080&q=75 1080w, /_next/image?url=%2Fwide.png&w=1200&q=75 1200w, /_next/image?url=%2Fwide.png&w=1920&q=75 1920w, /_next/image?url=%2Fwide.png&w=2048&q=75 2048w, /_next/image?url=%2Fwide.png&w=3840&q=75 3840w`
       )
 
-      expect(await browser.elementById('raw3').getAttribute('style')).toBe(
-        'aspect-ratio:400 / 400'
-      )
+      expect(await browser.elementById('raw3').getAttribute('style')).toBeNull()
       expect(await browser.elementById('raw3').getAttribute('srcset')).toBe(
         `/_next/image?url=%2Ftest.png&w=640&q=75 1x, /_next/image?url=%2Ftest.png&w=828&q=75 2x`
       )
+      if (mode === 'dev') {
+        await waitFor(1000)
+        const warnings = (await browser.log('browser'))
+          .map((log) => log.message)
+          .join('\n')
+        expect(warnings).toMatch(
+          /Image with src "\/wide.png" has either width or height modified, but not the other./gm
+        )
+        expect(warnings).not.toMatch(
+          /Image with src "\/test.png" has either width or height modified, but not the other./gm
+        )
+      }
     } finally {
       if (browser) {
         await browser.close()
@@ -760,7 +770,7 @@ function runTests(mode) {
         await browser
           .elementById('with-overlapping-styles-raw')
           .getAttribute('style')
-      ).toBe('width:10px;border-radius:10px;margin:15px;aspect-ratio:400 / 400')
+      ).toBe('width:10px;border-radius:10px;margin:15px')
       expect(
         await browser
           .elementById('without-styles-responsive')
@@ -770,7 +780,7 @@ function runTests(mode) {
       )
       expect(
         await browser.elementById('without-styles-raw').getAttribute('style')
-      ).toBe('aspect-ratio:400 / 400')
+      ).toBeNull()
 
       if (mode === 'dev') {
         await waitFor(1000)