fix(common): remove default for image width (#47082)

Previously NgOptimizedImage would default to requesting
an image at the width matching the width attribute in
the image HTML. While this works for width attrs that
match the intrinsic size of the image (e.g. responsive
images or images sized with CSS), this can be a sharp
edge for developers who use the width/height attributes
to set rendered size (i.e. instead of CSS, which one can
do for a fixed size image). In this case, defaulting to
the width attribute for the requested image would mean
requesting always at 1x quality, so screens with a DPR
 of 2+ get an image that is too small. Without a default
request width, the image served by the CDN would likely
be at intrinsic size, so 2x images would look correct.

This PR also updates the NgOptimizedImage sandbox and tests.

PR Close #47082

packages/common/src/directives/ng_optimized_image.ts

@@ -15,8 +15,9 @@ import {RuntimeErrorCode} from '../errors';
  * Config options recognized by the image loader function.
  */
 export interface ImageLoaderConfig {
+  // Name of the image to be added to the image request URL
   src: string;
-  quality?: number;
+  // Width of the requested image (to be used when generating srcset)
   width?: number;
 }
 
@@ -246,13 +247,10 @@ export class NgOptimizedImage implements OnInit, OnChanges, OnDestroy {
   }
 
   private getRewrittenSrc(): string {
-    const imgConfig = {
-      src: this.rawSrc,
-      // TODO: if we're going to support responsive serving, we don't want to request the width
-      // based solely on the intrinsic width (e.g. if it's on mobile and the viewport is smaller).
-      // The width would require pre-processing before passing to the image loader function.
-      width: this.width,
-    };
+    // ImageLoaderConfig supports setting a width property. However, we're not setting width here
+    // because if the developer uses rendered width instead of intrinsic width in the HTML width
+    // attribute, the image requested may be too small for 2x+ screens.
+    const imgConfig = {src: this.rawSrc};
     return this.imageLoader(imgConfig);
   }
 

packages/common/src/private_export.ts

@@ -6,6 +6,6 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-export {IMAGE_LOADER as ɵIMAGE_LOADER, NgOptimizedImage as ɵNgOptimizedImage} from './directives/ng_optimized_image';
+export {IMAGE_LOADER as ɵIMAGE_LOADER, ImageLoaderConfig as ɵImageLoaderConfig, NgOptimizedImage as ɵNgOptimizedImage} from './directives/ng_optimized_image';
 export {DomAdapter as ɵDomAdapter, getDOM as ɵgetDOM, setRootDomAdapter as ɵsetRootDomAdapter} from './dom_adapter';
 export {BrowserPlatformLocation as ɵBrowserPlatformLocation} from './location/platform_location';

packages/common/test/directives/ng_optimized_image_spec.ts

@@ -13,18 +13,6 @@ import {ComponentFixture, TestBed} from '@angular/core/testing';
 import {expect} from '@angular/platform-browser/testing/src/matchers';
 
 describe('Image directive', () => {
-  it('should set `src` to `rawSrc` value if image loader is not provided', () => {
-    setupTestingModule();
-
-    const template = '<img rawSrc="path/img.png" width="100" height="50">';
-    const fixture = createTestComponent(template);
-    fixture.detectChanges();
-
-    const nativeElement = fixture.nativeElement as HTMLElement;
-    const img = nativeElement.querySelector('img')!;
-    expect(img.src.endsWith('/path/img.png')).toBeTrue();
-  });
-
   it('should set `loading` and `fetchpriority` attributes before `src`', () => {
     // Only run this test in a browser since the Node-based DOM mocks don't
     // allow to override `HTMLImageElement.prototype.setAttribute` easily.
@@ -75,20 +63,6 @@ describe('Image directive', () => {
     expect(_fetchpriorityAttrId).toBeLessThan(_srcAttrId);  // was set after `src`
   });
 
-
-  it('should use an image loader provided via `IMAGE_LOADER` token', () => {
-    const imageLoader = (config: ImageLoaderConfig) => `${config.src}?w=${config.width}`;
-    setupTestingModule({imageLoader});
-
-    const template = '<img rawSrc="path/img.png" width="150" height="50">';
-    const fixture = createTestComponent(template);
-    fixture.detectChanges();
-
-    const nativeElement = fixture.nativeElement as HTMLElement;
-    const img = nativeElement.querySelector('img')!;
-    expect(img.src.endsWith('/path/img.png?w=150')).toBeTrue();
-  });
-
   describe('setup error handling', () => {
     it('should throw if both `src` and `rawSrc` are present', () => {
       setupTestingModule();
@@ -315,6 +289,54 @@ describe('Image directive', () => {
       expect(img.getAttribute('fetchpriority')).toBe('auto');
     });
   });
+
+  describe('loaders', () => {
+    it('should set `src` to match `rawSrc` if image loader is not provided', () => {
+      setupTestingModule();
+
+      const template = '<img rawSrc="path/img.png" width="100" height="50">';
+      const fixture = createTestComponent(template);
+      fixture.detectChanges();
+
+      const nativeElement = fixture.nativeElement as HTMLElement;
+      const img = nativeElement.querySelector('img')!;
+      expect(img.src.trim()).toBe('/path/img.png');
+    });
+
+    it('should set `src` using the image loader provided via the `IMAGE_LOADER` token to compose src URL',
+       () => {
+         const imageLoader = (config: ImageLoaderConfig) => `path/${config.src}`;
+         setupTestingModule({imageLoader});
+
+         const template = `
+        <img rawSrc="img.png" width="150" height="50">
+        <img rawSrc="img-2.png" width="150" height="50">
+      `;
+         const fixture = createTestComponent(template);
+         fixture.detectChanges();
+
+         const nativeElement = fixture.nativeElement as HTMLElement;
+         const imgs = nativeElement.querySelectorAll('img')!;
+         expect(imgs[0].src.trim()).toBe('/path/img.png');
+         expect(imgs[1].src.trim()).toBe('/path/img-2.png');
+       });
+
+    it('should set`src` to an image URL that does not include a default width parameter', () => {
+      const imageLoader = (config: ImageLoaderConfig) => {
+        const widthStr = config.width ? `?w=${config.width}` : ``;
+        return `path/${config.src}${widthStr}`;
+      };
+      setupTestingModule({imageLoader});
+
+      const template = '<img rawSrc="img.png" width="150" height="50">';
+      const fixture = createTestComponent(template);
+      fixture.detectChanges();
+
+      const nativeElement = fixture.nativeElement as HTMLElement;
+      const img = nativeElement.querySelector('img')!;
+      expect(img.src.trim()).toBe('/path/img.png');
+    });
+  });
 });
 
 // Helpers

packages/core/test/bundling/image-directive/BUILD.bazel

@@ -54,8 +54,6 @@ ts_devserver(
     static_files = [
         "index.html",
         ":tslib",
-        "a.png",
-        "b.png",
     ],
     deps = [":image-directive"],
 )

packages/core/test/bundling/image-directive/basic/basic.e2e-spec.ts

@@ -17,6 +17,6 @@ describe('NgOptimizedImage directive', () => {
     await browser.get('/');
     const imgs = element.all(by.css('img'));
     const src = await imgs.get(0).getAttribute('src');
-    expect(src.endsWith('b.png')).toBe(true);
+    expect(/a\.png/.test(src)).toBe(true);
   });
 });

packages/core/test/bundling/image-directive/basic/basic.ts

@@ -6,15 +6,51 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {ɵIMAGE_LOADER as IMAGE_LOADER, ɵNgOptimizedImage as NgOptimizedImage} from '@angular/common';
+import {ɵIMAGE_LOADER as IMAGE_LOADER, ɵImageLoaderConfig as ImageLoaderConfig, ɵNgOptimizedImage as NgOptimizedImage} from '@angular/common';
 import {Component} from '@angular/core';
 
+const CUSTOM_IMGIX_LOADER = (config: ImageLoaderConfig) => {
+  const widthStr = config.width ? `?w=${config.width}` : ``;
+  return `https://aurora-project.imgix.net/${config.src}${widthStr}`;
+};
+
+
 @Component({
   selector: 'basic',
+  styles: [`
+    h1 {
+      display: flex;
+      align-items: center;
+    }
+
+    main {
+      border: 1px solid blue;
+      margin: 16px;
+      padding: 16px;
+    }
+
+    .spacer {
+      height: 3000px;
+    }
+
+    main img {
+      width: 100%;
+      height: auto;
+    }
+  `],
+  template: `
+    <h1> 
+      <img rawSrc="a.png" width="50" height="50" priority>
+      <span>Angular image app</span>
+    </h1>
+    <main>
+      <div class="spacer"></div>
+      <img rawSrc="hermes.jpeg" width="4030" height="3020">
+    </main>
+  `,
   standalone: true,
   imports: [NgOptimizedImage],
-  template: `<img rawSrc="./a.png" width="150" height="150" priority>`,
-  providers: [{provide: IMAGE_LOADER, useValue: () => 'b.png'}],
+  providers: [{provide: IMAGE_LOADER, useValue: CUSTOM_IMGIX_LOADER}],
 })
 export class BasicComponent {
 }

packages/core/test/bundling/image-directive/index.ts

@@ -18,6 +18,7 @@ import {LcpCheckComponent} from './lcp-check/lcp-check';
   standalone: true,
   imports: [RouterModule],
   template: '<router-outlet></router-outlet>',
+
 })
 export class RootComponent {
 }