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

atcastle · 2022-04-27T19:07:26Z

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

ijjk · 2022-04-27T19:26:36Z

Failing test suites

Commit: 50aa4c5

yarn testheadless test/integration/image-component/default/test/index.test.js

Image Component Tests > dev mode > should handle the styles prop appropriately
Image Component Tests > server mode > should handle the styles prop appropriately
Image Component Tests > serverless mode > should handle the styles prop appropriately

Expand output

● Image Component Tests › dev mode › should handle the styles prop appropriately

expect(received).toBe(expected) // Object.is equality

Expected: "width:10px;border-radius:10px;margin:15px;"
Received: "width:10px;border-radius:10px;margin:15px"

  771 |           .elementById('with-overlapping-styles-raw')
  772 |           .getAttribute('style')
> 773 |       ).toBe('width:10px;border-radius:10px;margin:15px;')
      |         ^
  774 |       expect(
  775 |         await browser
  776 |           .elementById('without-styles-responsive')

  at Object.<anonymous> (integration/image-component/default/test/index.test.js:773:9)

● Image Component Tests › server mode › should handle the styles prop appropriately

expect(received).toBe(expected) // Object.is equality

Expected: "width:10px;border-radius:10px;margin:15px;"
Received: "width:10px;border-radius:10px;margin:15px"

  771 |           .elementById('with-overlapping-styles-raw')
  772 |           .getAttribute('style')
> 773 |       ).toBe('width:10px;border-radius:10px;margin:15px;')
      |         ^
  774 |       expect(
  775 |         await browser
  776 |           .elementById('without-styles-responsive')

  at Object.<anonymous> (integration/image-component/default/test/index.test.js:773:9)
      at runMicrotasks (<anonymous>)

● Image Component Tests › serverless mode › should handle the styles prop appropriately

expect(received).toBe(expected) // Object.is equality

Expected: "width:10px;border-radius:10px;margin:15px;"
Received: "width:10px;border-radius:10px;margin:15px"

  771 |           .elementById('with-overlapping-styles-raw')
  772 |           .getAttribute('style')
> 773 |       ).toBe('width:10px;border-radius:10px;margin:15px;')
      |         ^
  774 |       expect(
  775 |         await browser
  776 |           .elementById('without-styles-responsive')

  at Object.<anonymous> (integration/image-component/default/test/index.test.js:773:9)

Read more about building and testing Next.js in contributing.md.

ijjk · 2022-04-27T19:52:14Z

Stats from current PR

Default Build (Decrease detected ✓)

General Overall increase ⚠️

	vercel/next.js canary	atcastle/next.js raw-mode-height-width	Change
buildDuration	16s	15.8s	-165ms
buildDurationCached	6.1s	6.4s	⚠️ +219ms
nodeModulesSize	481 MB	481 MB	⚠️ +810 B

Page Load Tests Overall decrease ⚠️

	vercel/next.js canary	atcastle/next.js raw-mode-height-width	Change
/ failed reqs	0	0	✓
/ total time (seconds)	3.246	3.291	⚠️ +0.04
/ avg req/sec	770.26	759.57	⚠️ -10.69
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.384	1.408	⚠️ +0.02
/error-in-render avg req/sec	1805.82	1775.7	⚠️ -30.12

Client Bundles (main, webpack)

	vercel/next.js canary	atcastle/next.js raw-mode-height-width	Change
925.HASH.js gzip	179 B	179 B	✓
framework-HASH.js gzip	42 kB	42 kB	✓
main-HASH.js gzip	28.6 kB	28.6 kB	✓
webpack-HASH.js gzip	1.44 kB	1.44 kB	✓
Overall change	72.3 kB	72.3 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	atcastle/next.js raw-mode-height-width	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages Overall decrease ✓

	vercel/next.js canary	atcastle/next.js raw-mode-height-width	Change
_app-HASH.js gzip	1.36 kB	1.36 kB	✓
_error-HASH.js gzip	193 B	193 B	✓
amp-HASH.js gzip	308 B	308 B	✓
css-HASH.js gzip	327 B	327 B	✓
dynamic-HASH.js gzip	3.08 kB	3.08 kB	✓
head-HASH.js gzip	359 B	359 B	✓
hooks-HASH.js gzip	920 B	920 B	✓
image-HASH.js gzip	5.73 kB	5.71 kB	-21 B
index-HASH.js gzip	263 B	263 B	✓
link-HASH.js gzip	2.63 kB	2.63 kB	✓
routerDirect..HASH.js gzip	320 B	320 B	✓
script-HASH.js gzip	391 B	391 B	✓
withRouter-HASH.js gzip	318 B	318 B	✓
85e02e95b279..7e3.css gzip	107 B	107 B	✓
Overall change	16.3 kB	16.3 kB	-21 B

Client Build Manifests

	vercel/next.js canary	atcastle/next.js raw-mode-height-width	Change
_buildManifest.js gzip	459 B	459 B	✓
Overall change	459 B	459 B	✓

Rendered Page Sizes

	vercel/next.js canary	atcastle/next.js raw-mode-height-width	Change
index.html gzip	531 B	531 B	✓
link.html gzip	545 B	545 B	✓
withRouter.html gzip	526 B	526 B	✓
Overall change	1.6 kB	1.6 kB	✓

Diffs

Diff for _buildManifest.js

@@ -12,7 +12,7 @@ self.__BUILD_MANIFEST = {
   ],
   "/head": ["static\u002Fchunks\u002Fpages\u002Fhead-544bb68363445a0e.js"],
   "/hooks": ["static\u002Fchunks\u002Fpages\u002Fhooks-c1372eeb4916d32c.js"],
-  "/image": ["static\u002Fchunks\u002Fpages\u002Fimage-bfb178f7d6259a9d.js"],
+  "/image": ["static\u002Fchunks\u002Fpages\u002Fimage-75136b1767faacf3.js"],
   "/link": ["static\u002Fchunks\u002Fpages\u002Flink-b2c86b9b6041c9a3.js"],
   "/routerDirect": [
     "static\u002Fchunks\u002Fpages\u002FrouterDirect-113a7082ae16fbcb.js"

Diff for image-HASH.js

@@ -325,11 +325,7 @@
         var imgStyle = Object.assign(
           {},
           style,
-          layout === "raw"
-            ? {
-                aspectRatio: "".concat(widthInt, " / ").concat(heightInt)
-              }
-            : layoutStyle
+          layout === "raw" ? {} : layoutStyle
         );
         var blurStyle =
           placeholder === "blur" && !blurComplete
@@ -868,7 +864,7 @@
             });
           }
           if (false) {
-            var parent, ref3;
+            var parent, widthModified, heightModified, ref3;
           }
         });
       }
@@ -926,7 +922,7 @@
               {},
               rest,
               imgAttributes,
-              layout === "raw" && !imgAttributes.sizes
+              layout === "raw"
                 ? {
                     height: heightInt,
                     width: widthInt
@@ -1007,7 +1003,7 @@
                     sizes: imgAttributes.sizes,
                     loader: loader
                   }),
-                  layout === "raw" && !imgAttributes.sizes
+                  layout === "raw"
                     ? {
                         height: heightInt,
                         width: widthInt

Default Build with SWC (Decrease detected ✓)

General Overall increase ⚠️

	vercel/next.js canary	atcastle/next.js raw-mode-height-width	Change
buildDuration	18.1s	18.2s	⚠️ +55ms
buildDurationCached	6.2s	6.2s	-26ms
nodeModulesSize	481 MB	481 MB	⚠️ +810 B

Page Load Tests Overall decrease ⚠️

	vercel/next.js canary	atcastle/next.js raw-mode-height-width	Change
/ failed reqs	0	0	✓
/ total time (seconds)	3.301	3.315	⚠️ +0.01
/ avg req/sec	757.39	754.14	⚠️ -3.25
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.387	1.386	0
/error-in-render avg req/sec	1802.66	1803.58	+0.92

Client Bundles (main, webpack)

	vercel/next.js canary	atcastle/next.js raw-mode-height-width	Change
925.HASH.js gzip	178 B	178 B	✓
framework-HASH.js gzip	42.2 kB	42.2 kB	✓
main-HASH.js gzip	29.1 kB	29.1 kB	✓
webpack-HASH.js gzip	1.45 kB	1.45 kB	✓
Overall change	72.9 kB	72.9 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	atcastle/next.js raw-mode-height-width	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages Overall decrease ✓

	vercel/next.js canary	atcastle/next.js raw-mode-height-width	Change
_app-HASH.js gzip	1.35 kB	1.35 kB	✓
_error-HASH.js gzip	179 B	179 B	✓
amp-HASH.js gzip	312 B	312 B	✓
css-HASH.js gzip	324 B	324 B	✓
dynamic-HASH.js gzip	3.08 kB	3.08 kB	✓
head-HASH.js gzip	357 B	357 B	✓
hooks-HASH.js gzip	921 B	921 B	✓
image-HASH.js gzip	5.78 kB	5.76 kB	-17 B
index-HASH.js gzip	261 B	261 B	✓
link-HASH.js gzip	2.74 kB	2.74 kB	✓
routerDirect..HASH.js gzip	322 B	322 B	✓
script-HASH.js gzip	392 B	392 B	✓
withRouter-HASH.js gzip	317 B	317 B	✓
85e02e95b279..7e3.css gzip	107 B	107 B	✓
Overall change	16.4 kB	16.4 kB	-17 B

Client Build Manifests Overall increase ⚠️

	vercel/next.js canary	atcastle/next.js raw-mode-height-width	Change
_buildManifest.js gzip	456 B	457 B	⚠️ +1 B
Overall change	456 B	457 B	⚠️ +1 B

Rendered Page Sizes

	vercel/next.js canary	atcastle/next.js raw-mode-height-width	Change
index.html gzip	527 B	527 B	✓
link.html gzip	541 B	541 B	✓
withRouter.html gzip	525 B	525 B	✓
Overall change	1.59 kB	1.59 kB	✓

Diffs

Diff for _buildManifest.js

@@ -12,7 +12,7 @@ self.__BUILD_MANIFEST = {
   ],
   "/head": ["static\u002Fchunks\u002Fpages\u002Fhead-544bb68363445a0e.js"],
   "/hooks": ["static\u002Fchunks\u002Fpages\u002Fhooks-c1372eeb4916d32c.js"],
-  "/image": ["static\u002Fchunks\u002Fpages\u002Fimage-bfb178f7d6259a9d.js"],
+  "/image": ["static\u002Fchunks\u002Fpages\u002Fimage-75136b1767faacf3.js"],
   "/link": ["static\u002Fchunks\u002Fpages\u002Flink-b2c86b9b6041c9a3.js"],
   "/routerDirect": [
     "static\u002Fchunks\u002Fpages\u002FrouterDirect-113a7082ae16fbcb.js"

Diff for image-HASH.js

@@ -325,11 +325,7 @@
         var imgStyle = Object.assign(
           {},
           style,
-          layout === "raw"
-            ? {
-                aspectRatio: "".concat(widthInt, " / ").concat(heightInt)
-              }
-            : layoutStyle
+          layout === "raw" ? {} : layoutStyle
         );
         var blurStyle =
           placeholder === "blur" && !blurComplete
@@ -868,7 +864,7 @@
             });
           }
           if (false) {
-            var parent, ref3;
+            var parent, widthModified, heightModified, ref3;
           }
         });
       }
@@ -926,7 +922,7 @@
               {},
               rest,
               imgAttributes,
-              layout === "raw" && !imgAttributes.sizes
+              layout === "raw"
                 ? {
                     height: heightInt,
                     width: widthInt
@@ -1007,7 +1003,7 @@
                     sizes: imgAttributes.sizes,
                     loader: loader
                   }),
-                  layout === "raw" && !imgAttributes.sizes
+                  layout === "raw"
                     ? {
                         height: heightInt,
                         width: widthInt

Commit: a7bada0

styfle · 2022-04-27T20:06:16Z

docs/api-reference/next/image.md

@@ -95,7 +95,6 @@ The layout behavior of the image as the viewport changes size.
  - 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.


I think this sentence needs be to adjusted?

Maybe this sentence is a little awkward, but I think it's basically still correct. If you don't include sizes, the srcset works like a fixed image (1x and 2x only). I think "receive" is the confusing word here. I'll see if I can come up with something else.

Changed it to: If your image styling will change the size of a rawimage, you should include thesizes property for proper image serving. Otherwise your image will be requested as though it has fixed height and width.

docs/api-reference/next/image.md

packages/next/client/image.tsx

styfle · 2022-04-27T20:13:05Z

test/integration/image-component/default/pages/layout-raw.js

+          id="raw4"
+          src="/test.png"
+          width="400"
+          height="400"


Should we also allow the scenario where width/height are only in the style, not the actual props?

@cramforce Thoughts?

I'm not sure the added flexibility there would be worth the increase in complexity in the code. Plus, if you're allowed to provide height and width via the style prop, it raises the question why can't you provide it via other sources of styling like a CSS file or whatever.

It would be difficult to detect. I think the width/height attributes should be validated to be integers, and then people can go nuts with the CSS.

For sure. I've got another PR after this one to add extra validation on height and width, plus tests for that.

cramforce · 2022-04-27T20:18:17Z

packages/next/client/image.tsx

+          (!heightModified && widthModified)
+        ) {
+          warnOnce(
+            `Image with src ${src} has either height or width modified, but not the other. If you use CSS to change the size of your image, use height="auto" or width="auto" to maintain the aspect ratio.`


This is conflating CSS and HTML, right? Width and height should always be integers and the height: auto should be in CSS.

Yes, maybe it's not clear enough. Since the first part of the warning specifies CSS, I intended that "use height="auto" would be understood in the context of CSS, too. I'll see if I can clarify that.

Ah, I realize the = makes it look like an attribute. I've updated it to Image with src ${src} has either height or width modified, but not the other. If you use CSS to change the size of your image, also include the styles 'height: "auto"' or 'width: "auto"' to maintain the aspect ratio.

cramforce · 2022-04-27T20:19:53Z

test/integration/image-component/default/pages/layout-raw.js

+          id="raw4"
+          src="/test.png"
+          width="400"
+          height="400"


It would be difficult to detect. I think the width/height attributes should be validated to be integers, and then people can go nuts with the CSS.

docs/api-reference/next/image.md

Co-authored-by: Steven <steven@ceriously.com>

packages/next/client/image.tsx

test/integration/image-component/default/test/index.test.js

Co-authored-by: Steven <steven@ceriously.com>

styfle

Thanks!

Alex Castle added 4 commits April 26, 2022 15:22

Always add height and width in raw mode. Warn on stretched image

7739eb6

Add tests for stretched image warning

9a2ded5

Update raw image documentation

408e40a

remove unnecessary aspect-ratio styling for raw layout

50aa4c5

atcastle requested review from timneutkens, ijjk, shuding, styfle, huozhi, padmaia, leerob and balazsorban44 as code owners April 27, 2022 19:07

ijjk added area: documentation created-by: Chrome Aurora PRs by the Google Chrome team: https://web.dev/aurora type: next labels Apr 27, 2022

atcastle changed the title ~~Always dd height and width prop to image with layout="raw"~~ Always add height and width prop to image with layout="raw" Apr 27, 2022

Test fix

ddb141d