Skip to content

Commit

Permalink
fix(common): don't generate srcset if noopImageLoader is used (#47804)
Browse files Browse the repository at this point in the history
Do not generate a srcset if the loader being used is the default noopImageLoader. This loader does not take width into account, so it does not make sense to use it with srcsets.

PR Close #47804
  • Loading branch information
atcastle authored and dylhunn committed Oct 19, 2022
1 parent 716352c commit 4f52d4e
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 3 deletions.
Expand Up @@ -418,7 +418,8 @@ export class NgOptimizedImage implements OnInit, OnChanges, OnDestroy {

if (this.ngSrcset) {
rewrittenSrcset = this.getRewrittenSrcset();
} else if (!this._disableOptimizedSrcset && !this.srcset) {
} else if (
!this._disableOptimizedSrcset && !this.srcset && this.imageLoader !== noopImageLoader) {
rewrittenSrcset = this.getAutomaticSrcset();
}

Expand Down
19 changes: 17 additions & 2 deletions packages/common/test/directives/ng_optimized_image_spec.ts
Expand Up @@ -1401,6 +1401,20 @@ describe('Image directive', () => {
return `${IMG_BASE_URL}/${config.src}${width}`;
};

it('should not generate a srcset if the default noop loader is used', () => {
setupTestingModule({noLoader: true});

const template = `
<img ngSrc="img" width="100" height="50" sizes="100vw">
`;
const fixture = createTestComponent(template);
fixture.detectChanges();

const nativeElement = fixture.nativeElement as HTMLElement;
const img = nativeElement.querySelector('img')!;
expect(img.getAttribute('srcset')).toBeNull();
});

it('should add a responsive srcset to the img element if sizes attribute exists', () => {
setupTestingModule({imageLoader});

Expand Down Expand Up @@ -1538,6 +1552,7 @@ class TestComponent {
function setupTestingModule(config?: {
imageConfig?: ImageConfig,
imageLoader?: ImageLoader,
noLoader?: boolean,
extraProviders?: Provider[],
component?: Type<unknown>
}) {
Expand All @@ -1548,8 +1563,8 @@ function setupTestingModule(config?: {
const loader = config?.imageLoader || defaultLoader;
const extraProviders = config?.extraProviders || [];
const providers: Provider[] = [
{provide: DOCUMENT, useValue: window.document}, {provide: IMAGE_LOADER, useValue: loader},
...extraProviders
{provide: DOCUMENT, useValue: window.document},
...(config?.noLoader ? [] : [{provide: IMAGE_LOADER, useValue: loader}]), ...extraProviders
];
if (config?.imageConfig) {
providers.push({provide: IMAGE_CONFIG, useValue: config.imageConfig});
Expand Down

0 comments on commit 4f52d4e

Please sign in to comment.